4206909 - adding Z_SYNC_FLUSH support to deflaters

[Xueming Shen]

Martin, thanks for the comments.

webrev has been updated accordingly, I will submit it for the CCC.

What should the "google copyright" look like for InflateIn_DeflateOut.java?

I picked it from

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/Z_SYNC_FLUSH/test/java/util/zip/InflateIn_DeflateOut.java.html

which uses the sun copyright, I guess it was copied from the your 
DeflateIn_InflateOut.java test case.

So if you can pass me your google version I will paste it in. I assume 
It should be OK that I then put the
sun one on top of that.

Glad to see the light at the end of the tunnel finally:-)

sherman

Martin Buchholz wrote:
> [+ Brandon]
>
> Sherman,
> Here are some review comments on "take1"
>
> Overall, looks good.
> This low-level API will be annoying to use,
> but that's mostly unavoidable.
>
> ----
> typos: @linke sinze
> ----
> source files contain TABs, no longer permitted
> ----
> <tt> is denigrated in favor of @code
> ----
> I don't think you really meant to include ./Flush.java in the webrev
> ----
> {@link Deflater.SYNC_FLUSH}
> =>
> {@link Deflater#SYNC_FLUSH}
> ----
> Please use a Google copyright in InflateIn_DeflateOut
> ----
>
> Martin
>
> On Tue, Sep 22, 2009 at 13:03, Xueming Shen <Xueming.Shen at sun.com> wrote:
>   
>> Thanks Alan!
>>
>> The webrev has been updated accordingly to address your comments (use
>> syncFlush instead of
>> doSyncFlush and those suggestions for Deflater.java).
>>
>> http://cr.openjdk.java.net/~sherman/zipflush/webrev
>>
>> Sherman
>>
>> PS. There was a "take2" for DOS that I think might be more consistent with
>> the existing APIs, but it might be the
>> time to focus on one approach. If you're interested the "take2" is at
>>
>> http://cr.openjdk.java.net/~sherman/zipflush/webrev.take2/src/share/classes/java/util/zip/DeflaterOutputStream.java.sdiff.html
>>
>> Alan Bateman wrote:
>>     
>>> Welcome back (for some reason I thought you were gone for two weeks).
>>>
>>> It would be best to send the proposal to the mailing list so that others
>>> can comment.
>>>
>>> Personally, I don't like exposing the flush mode to subclasses but you are
>>> right that it is more consistent with the original design.  If this approach
>>> is chosen then I would suggest that the flush implementation make a copy of
>>> the flush mode before testing and using it. Also, the new constructors will
>>> need to say that they initialize the flush mode based on the parameter
>>> (since it can change on the fly). BTW: What is the reason for renaming the
>>> parameter to flushDef. I prefer "syncFlush" over "doSyncFlush".
>>>
>>> I'm happy with Deflater. In the description of SYNC_FLUSH I would suggest
>>> changing the "," into a ";" or else make it into two sentences. In the new
>>> deflate method the description of NO_FLUSH needs a comma after "accumulate"
>>> for it to flow well. Similar issue in the SYNC_FLUSH description where you
>>> need a comma after "is flushed". There is also a typo [ comparssed :-) ]
>>>
>>> -Alan.
>>>
>>>
>>>       
>>