Code review request for 4206909 : adding Z_SYNC_FLUSH support to deflaters (original) (raw)

Alan Bateman Alan.Bateman at Sun.COM
Wed Oct 21 13:22:33 UTC 2009

Xueming Shen wrote:

Martin, Alan.

Finally got the CCC approval. Here is the final webrev http://cr.openjdk.java.net/~sherman/4206909/webrev The only difference compared to the webrev you guys reviewed last time at http://cr.openjdk.java.net/~sherman/zipflush/webrev is to use the Google copyright in test case InflateInDeflateOut.java Thanks, Sherman Good to see this one coming near to the finish. The changes mostly look good to me and the test looks good (thanks Martin). One comment is that DeflaterOutputStream.syncFlush be final. Other than that, I only have a few minor nits:

  1. In DeflaterOutputStream.flush() it would be nicer if the test was: if (syncFlush && !def.finished()) { ... } That might the line wrap in the middle of the while expression.

  2. Should you use @throws instead of @exception? (I haven't seen the latter in several years).

  3. Do you need the blank line between the @see and @since tags in Deflater's constants? (seems locally inconsistent).

-Alan.

More information about the core-libs-dev mailing list