Code review request for 4206909 : adding Z_SYNC_FLUSH support to deflaters
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 InflateIn_DeflateOut.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