Re: [compress] Dave's Code Review (was Re: [CANCELLED][VOTE] Release Commons Compress 1.6)

2013-10-15 Thread Stefan Bodewig
On 2013-10-15, Stefan Bodewig wrote: > But we probably should store the password as a char[] anyway s/char/byte/ Stefan - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@comm

[compress] Dave's Code Review (was Re: [CANCELLED][VOTE] Release Commons Compress 1.6)

2013-10-15 Thread Stefan Bodewig
Hi I'm going to address Dave's three mails in a single response dam6923 . wrote: > In SevenZFile.java > > Constructor... > 1) Close file on exception instead of the current technique of keeping > a "succeeded" flag. This means I have to catch and rethrow the exception. I don't think I like th

Re: [CANCELLED][VOTE] Release Commons Compress 1.6

2013-10-14 Thread dam6923 .
org.apache.commons.compress.archivers.ar.ArArchiveInputStream#getNextArEntry() The code to "skip to the next available entry" looks a bit inefficient because it reads one byte at a time in a loop. Consider using org.apache.commons.compress.utils.IOUtils.skip(InputStream, long) Consider using org

Re: [CANCELLED][VOTE] Release Commons Compress 1.6

2013-10-14 Thread Gary Gregory
On Mon, Oct 14, 2013 at 12:03 PM, dam6923 . wrote: > Just a couple of immediate thoughts (just starting to look things over...) > > In SevenZFile.java > > Constructor... > 1) Close file on exception instead of the current technique of keeping > a "succeeded" flag. > 2) Move setting the password to

Re: [CANCELLED][VOTE] Release Commons Compress 1.6

2013-10-14 Thread dam6923 .
org.apache.commons.compress.compressors.lzma.LZMACompressorInputStream#read() Current: count(ret == -1 ? -1 : 1); Change: count(ret == -1 ? 0 : 1); On Mon, Oct 14, 2013 at 12:17 AM, Stefan Bodewig wrote: > I think enough issues have been identified to warrant a second RC. > > I'll take care of t

Re: [CANCELLED][VOTE] Release Commons Compress 1.6

2013-10-14 Thread dam6923 .
Just a couple of immediate thoughts (just starting to look things over...) In SevenZFile.java Constructor... 1) Close file on exception instead of the current technique of keeping a "succeeded" flag. 2) Move setting the password to the last line of the constructor so that it isn't stored if an ex

[CANCELLED][VOTE] Release Commons Compress 1.6

2013-10-13 Thread Stefan Bodewig
I think enough issues have been identified to warrant a second RC. I'll take care of the bad version number in the release notes as well as the PMD warnings in the ARJ-Package. ArjArchiveEntry's test coverage has already been increased in trunk. We should talk about the Clirr report further and