Hi Stefan,
(...) > read never indicates EOF as it stands, I think we should return -1 > rather than 0 when position equals size. WDYT? Yes, indeed the contract specifies to return -1 in this case so we should change this. >> In resize() method there is also a danger to overflow integer when we are >> close to Integer.MAX_VALUE - either sum pos + wanted or shift may cause this. > True. One fix would be to ensure position + wanted is cut to > Integer.MAX_VALUE in write (in case the sum turns negative) and in > resize set len to newLength if newLength > Integer.MAX_VALUE / 2. Allocation is an interesting topic in itself. I have looked at ArrayList allocation algorithm and I think they got it right. First there is more conservative approach to allocation growth - 1.5 ratio instead of 2.0 as we use. Also there is an overflow handled. Take a look at ArrayList.grow() method. The question is how to use this approach not to violate any license/intellectual property? In the meantime I have created a patch with additional constructors, javadocs and also simple examples for in-memory cases . Patch is attached to Jira issue. Take a look, agree or disagree, thanks. Cheers, Maciej ________________________________ From: Stefan Bodewig <bode...@apache.org> Sent: Saturday, October 22, 2016 4:46:34 PM To: dev@commons.apache.org Subject: Re: [compress] Added in-memory support for zip and 7z Hi Maciej patch applied. On 2016-10-22, M N wrote: > Going back to the fix - first I've done the homework and read the contract of > SeekableByteChannel.position(long) method. > It influences read() and write() operation. > Citation of the most important part: > "Setting the position to a value that is greater than the current size is > legal but does not change the size of the entity. A later attempt to read > bytes at such a position will immediately return an end-of-file indication. A > later attempt to write bytes at such a position will cause the entity to grow > to accommodate the new bytes; the values of any bytes between the previous > end-of-file and the newly-written bytes are unspecified." > My changes to SeekableInMemoryByteChannel: > 1. In read() method added check whether position exceeds size. > 2. Added check for negative argument in position(long). > 3. Added check for open channel in position(long) to fail immediately. In > general the contract specifies to throw ClosedChannelExceptionexception by > each method. Thanks, I overlooked 1 and 3 and didn't really think about 2. read never indicates EOF as it stands, I think we should return -1 rather than 0 when position equals size. WDYT? > In resize() method there is also a danger to overflow integer when we are > close to Integer.MAX_VALUE - either sum pos + wanted or shift may cause this. True. One fix would be to ensure position + wanted is cut to Integer.MAX_VALUE in write (in case the sum turns negative) and in resize set len to newLength if newLength > Integer.MAX_VALUE / 2. Stefan --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
