[JDK-6341887] RFR: Patch V3: java.util.zip: Add ByteBuffer methods to Inflater/Deflater

[Alan Bateman]

Is there an up to date webrev on cr.openjdk.java.net that we can review? 
The inflate/deflate methods in the original proposal made sense, I'm 
less sure about introducing a new setDictionary method.

-Alan

On 23/03/2018 17:51, Xueming Shen wrote:
>
> Hi David,
>
> Sorry for the late response. I will sponsor this change. Will prepare 
> for the CSR for
> the new APIs.
>
> Couple more comments
>
> (1) Deflater.java/deflate(ByteBuffer, int);
> "In the case of FULL_FLUSH or SYNC_FLUSH ..."
>
> It's a copy/paste from the corresponding part in deflate(byte[], int, 
> int, int). The existing
> one might not be clear. What it meant to say is "if the returned value 
> is equal to the
> space available in the output buffer passed in ...then you need to 
> call it with more sapce,
> greater than 6 bytes ...". So the {@code len} need to updated to 
> something like
> {@code output.remainint()}.
>
> (2) Inflater.java/setDictionary(ByteBuffer);
> "Sets the preset dictionary to the given array of bytes"
>     --> bytes in the specified byte buffer?
>
> (3) Infalter.java.inflater(...)
> It appears there is opportunity to "consolidate" some of the 
> repetitive code block,
> for example the "catch (DFE e) { ...} the only difference is the 
> "inputPos/input.position()"
> line. Maybe we can move the try/catch out a "level" and consolidate 
> this catch block.
> Sure it's a style preference. We can deal with it later.
>
> Thanks,
> Sherman
>
>
> On 03/23/2018 08:18 AM, David Lloyd wrote:
>> Are there any further comments on this? If not, would it be possible
>> to get a sponsor for this change?
>>
>> Sorry again for the detached email threads; I've learned my GMail 
>> lesson well...
>>
>> Thanks.
>>
>> On Fri, Mar 16, 2018 at 8:25 AM, David Lloyd<david.lloyd at redhat.com>  
>> wrote:
>>> Sorry, that was an error on my part, caused by too much context
>>> switching.  I've posted an update at
>>> https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v13 which is
>>> also attached.
>>>
>>> On Wed, Mar 14, 2018 at 8:53 PM, Xueming 
>>> Shen<xueming.shen at oracle.com>  wrote:
>>>> Hi David,
>>>>
>>>> https://github.com/dmlloyd/openjdk/commit/zlib-bytebuffer-v12
>>>>
>>>> Should we start to review the changes included in above link, or we 
>>>> should
>>>> wait ? It appears
>>>> the API is being updated but some implementation have not been 
>>>> updated to
>>>> follow the spec
>>>> yet, especially the piece that deals with the output 
>>>> buffer/byteWritten when
>>>> DataFormatException
>>>> is raised, for example
>>>>
>>>> (1) the "outputConsumedID" is defined but never used to update the
>>>> corresponding java field
>>>>       in Inflater.c and
>>>>
>>>> (2) the "outputConsumed" is used to update the output ByteBuffer 
>>>> when DFE
>>>> raised (in Java), but
>>>>       the corresponding "byteWritten" is not being updated before the
>>>> exception is thrown.
>>>>
>>>> -Sherman
>>>>
>>>>
>>>>
>>>>
>>>> On 3/13/18, 10:46 AM, David Lloyd wrote:
>>>>> Sorry all, it looks like GMail doesn't know how to keep replies with
>>>>> the thread when you change the subject line.  The follow-up to this
>>>>> thread is
>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/051960.html 
>>>>>
>>>>> with only a few small changes as discussed above.
>>>>>
>>>>> On Fri, Mar 2, 2018 at 2:36 PM, David Lloyd<david.lloyd at redhat.com>
>>>>> wrote:
>>>>>> On Fri, Mar 2, 2018 at 2:34 PM, David Lloyd<david.lloyd at redhat.com>
>>>>>> wrote:
>>>>>>> On Fri, Mar 2, 2018 at 12:49 PM, Xueming 
>>>>>>> Shen<xueming.shen at oracle.com>
>>>>>>> wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> (1) Deflater.deflate(Bytebuffer)
>>>>>>>>        the api doc regarding "no_flush" appears to be the 
>>>>>>>> copy/paste of
>>>>>>>> the
>>>>>>>> byte[] version
>>>>>>>>        without being updated to the corresponding ByteBuffer?
>>>>>>> You're right, I missed that one.  I've incorporated this fix 
>>>>>>> locally:
>>>>>> Oops, this should have been:
>>>>>>
>>>>>> --- 8<   --- cut here --- 8<   ---
>>>>>>
>>>>>> diff --git a/src/java.base/share/classes/java/util/zip/Deflater.java
>>>>>> b/src/java.base/share/classes/java/util/zip/Deflater.java
>>>>>> index 524125787a8..40f0d9736e2 100644
>>>>>> --- a/src/java.base/share/classes/java/util/zip/Deflater.java
>>>>>> +++ b/src/java.base/share/classes/java/util/zip/Deflater.java
>>>>>> @@ -481,9 +481,9 @@ public class Deflater {
>>>>>>         * in order to determine if more input data is required.
>>>>>>         *
>>>>>>         *<p>This method uses {@link #NO_FLUSH} as its compression 
>>>>>> flush
>>>>>> mode.
>>>>>> -     * An invocation of this method of the form {@code
>>>>>> deflater.deflate(b)}
>>>>>> +     * An invocation of this method of the form {@code
>>>>>> deflater.deflate(output)}
>>>>>>         * yields the same result as the invocation of
>>>>>> -     * {@code deflater.deflate(b, 0, b.length, Deflater.NO_FLUSH)}.
>>>>>> +     * {@code deflater.deflate(output, Deflater.NO_FLUSH)}.
>>>>>>         *
>>>>>>         * @param output the buffer for the compressed data
>>>>>>         * @return the actual number of bytes of compressed data 
>>>>>> written to
>>>>>> the
>>>>>>
>>>>>> --- 8<   --- cut here --- 8<   ---
>>>>>>
>>>>>> -- 
>>>>>> - DML
>>>>>
>>>>>
>>>
>>>
>>> -- 
>>> - DML
>>
>>
>