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