commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From M N <mach...@hotmail.com>
Subject Re: [compress] Added in-memory support for zip and 7z
Date Tue, 01 Nov 2016 21:41:56 GMT
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 <bodewig@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message