RFR: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary [v3]

Lance Andersen lancea at openjdk.java.net
Mon Sep 21 19:09:46 UTC 2020


On Mon, 21 Sep 2020 12:18:40 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> Update to Deflater.c looks good.
> 
> DeflaterDictionaryTests looks like is a shaping up to be a good test for setDictionary. Are there other assertions that
> should be checked, e.g. setDictionary(ByteBuffer) is specified to consume all bytes and it would be good to check that
> the position is set to the limit. Also can the 2-arg setDictionary be tests, also corner cases such no bytes remaining
> or invoked with null.

I have logged JDK-8253444 to add more coverage for setDictionary.  I would prefer to put this change back to address
the offset not being used and improve the coverage afterwards.

> The naming of the tests is a bit inconsistent, e.g. ByteArrayTest vs. testByteBufferDirect. In the naming then I would
> use "Heap" instead of "Wrap", as in testHeapByteBuffer.

I  updated the test name(s).
> 
> In passing: The tests can use try-finally to ensure that Deflater::end is invoked even when the test fails. String
> repeat(int) could be used to avoid duplicating "Welcome to Us Open;". Missing space in several "if(...)" usages.

Addressed the issues above.

I have pushed the updated changes for review to the PR.

-------------

PR: https://git.openjdk.java.net/jdk/pull/269


More information about the core-libs-dev mailing list