6341887: Inflater can't handle ByteBuffer - first webrev

[Alan Bateman]

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