6341887: Inflater can't handle ByteBuffer (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Sat Apr 14 21:15:34 UTC 2012

On 14/04/2012 19:29, Xueming Shen wrote:

Hi Martin,

Thanks for taking on this one. Here are my comments after first scan (1) Upon return, the position of the ByteBuffer should not always be updated to the "limit". It should depend on the number of bytes really compressed/de-compressed. Like the buffer at ReadableByteChannel.read(buffer). (2) The implementation of the native buffer version and non-buffer version probably can share most of their code in a separated method (3) The input part will be tough. I was struggling with if we should have a totally separated subclass, like DeflaterBuffer/InfalterBuffer (or BufferDefalter/Inflater) to only handle everything in ByteBuffer with methods handles buffer input and output, throw "not supported operation" for those "byte[]" methods. Otherwise you will have to put something in the specification to mandate the behavior of mixed bytebuff and byte[] scenario. I'm not sure which way is more appropriate though. -Sherman Sherman - I'll assume you'll sponsor Martin once he's signed up to contribute.

I didn't go through the patch in detail but I did notice that the buffer position should be pos + number of bytes written to buffer. Also the ArrayIndexOutOfBoundsException doesn't look right as it's not testable and means the invariant defined by Buffer is violated. In other places its an assert. A test case will also be needed, I didn't see that in the patch.

I didn't set new setInput methods in the patch but I agree that would require much more consideration.

-Alan

More information about the core-libs-dev mailing list