some of the findbugs issues are a little too severe. Do we really need to copy the arrays??
Peter On Thu, Feb 5, 2009 at 12:30 PM, <bode...@apache.org> wrote: > Author: bodewig > Date: Thu Feb 5 12:30:01 2009 > New Revision: 741089 > > URL: http://svn.apache.org/viewvc?rev=741089&view=rev > Log: > fix a bunch of findbugs reported problems in the zip, tar and bzip2 classes. > PR 46661 > > Modified: > ant/core/trunk/src/main/org/apache/tools/bzip2/CBZip2OutputStream.java > ant/core/trunk/src/main/org/apache/tools/tar/TarInputStream.java > ant/core/trunk/src/main/org/apache/tools/tar/TarOutputStream.java > ant/core/trunk/src/main/org/apache/tools/zip/AsiExtraField.java > ant/core/trunk/src/main/org/apache/tools/zip/UnrecognizedExtraField.java > ant/core/trunk/src/main/org/apache/tools/zip/ZipFile.java > ant/core/trunk/src/main/org/apache/tools/zip/ZipLong.java > ant/core/trunk/src/main/org/apache/tools/zip/ZipShort.java > ant/core/trunk/src/tests/junit/org/apache/tools/zip/AsiExtraFieldTest.java > ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipLongTest.java > ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipShortTest.java > > Modified: > ant/core/trunk/src/main/org/apache/tools/bzip2/CBZip2OutputStream.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/bzip2/CBZip2OutputStream.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/bzip2/CBZip2OutputStream.java > (original) > +++ ant/core/trunk/src/main/org/apache/tools/bzip2/CBZip2OutputStream.java > Thu Feb 5 12:30:01 2009 > @@ -613,7 +613,7 @@ > } > > if (ge > gs && nPart != nGroups && nPart != 1 > - && ((nGroups - nPart) % 2 == 1)) { > + && ((nGroups - nPart) % 2 != 0)) { > aFreq -= mtfFreq[ge]; > ge--; > } > @@ -983,9 +983,7 @@ > b = t; > } > if (b > c) { > - t = b; > b = c; > - c = t; > } > if (a > b) { > b = a; > @@ -1030,7 +1028,7 @@ > > med = med3(block[zptr[lo] + d + 1], > block[zptr[hi ] + d + 1], > - block[zptr[(lo + hi) >> 1] + d + 1]); > + block[zptr[(lo + hi) >>> 1] + d + 1]); > > unLo = ltLo = lo; > unHi = gtHi = hi; > > Modified: ant/core/trunk/src/main/org/apache/tools/tar/TarInputStream.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/tar/TarInputStream.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/tar/TarInputStream.java > (original) > +++ ant/core/trunk/src/main/org/apache/tools/tar/TarInputStream.java Thu Feb > 5 12:30:01 2009 > @@ -218,8 +218,13 @@ > + numToSkip + " bytes"); > } > > - if (numToSkip > 0) { > - skip(numToSkip); > + while (numToSkip > 0) { > + long skipped = skip(numToSkip); > + if (skipped <= 0) { > + throw new RuntimeException("failed to skip current tar" > + + " entry"); > + } > + numToSkip -= skipped; > } > > readBuf = null; > > Modified: ant/core/trunk/src/main/org/apache/tools/tar/TarOutputStream.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/tar/TarOutputStream.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/tar/TarOutputStream.java > (original) > +++ ant/core/trunk/src/main/org/apache/tools/tar/TarOutputStream.java Thu Feb > 5 12:30:01 2009 > @@ -310,7 +310,7 @@ > > wOffset += numToWrite; > assemLen += numToWrite; > - numToWrite -= numToWrite; > + numToWrite = 0; > } > } > > > Modified: ant/core/trunk/src/main/org/apache/tools/zip/AsiExtraField.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/zip/AsiExtraField.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/zip/AsiExtraField.java (original) > +++ ant/core/trunk/src/main/org/apache/tools/zip/AsiExtraField.java Thu Feb > 5 12:30:01 2009 > @@ -334,4 +334,14 @@ > return type | (mode & PERM_MASK); > } > > + public Object clone() { > + try { > + AsiExtraField cloned = (AsiExtraField) super.clone(); > + cloned.crc = new CRC32(); > + return cloned; > + } catch (CloneNotSupportedException cnfe) { > + // impossible > + throw new RuntimeException(cnfe); > + } > + } > } > > Modified: > ant/core/trunk/src/main/org/apache/tools/zip/UnrecognizedExtraField.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/zip/UnrecognizedExtraField.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/zip/UnrecognizedExtraField.java > (original) > +++ ant/core/trunk/src/main/org/apache/tools/zip/UnrecognizedExtraField.java > Thu Feb 5 12:30:01 2009 > @@ -65,7 +65,7 @@ > * @param data the field data to use > */ > public void setLocalFileDataData(byte[] data) { > - localData = data; > + localData = copy(data); > } > > /** > @@ -81,7 +81,7 @@ > * @return the local data > */ > public byte[] getLocalFileDataData() { > - return localData; > + return copy(localData); > } > > /** > @@ -97,7 +97,7 @@ > * @param data the data to use > */ > public void setCentralDirectoryData(byte[] data) { > - centralData = data; > + centralData = copy(data); > } > > /** > @@ -118,7 +118,7 @@ > */ > public byte[] getCentralDirectoryData() { > if (centralData != null) { > - return centralData; > + return copy(centralData); > } > return getLocalFileDataData(); > } > @@ -134,4 +134,13 @@ > System.arraycopy(data, offset, tmp, 0, length); > setLocalFileDataData(tmp); > } > + > + private static byte[] copy(byte[] from) { > + if (from != null) { > + byte[] to = new byte[from.length]; > + System.arraycopy(from, 0, to, 0, to.length); > + return to; > + } > + return null; > + } > } > > Modified: ant/core/trunk/src/main/org/apache/tools/zip/ZipFile.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/zip/ZipFile.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/zip/ZipFile.java (original) > +++ ant/core/trunk/src/main/org/apache/tools/zip/ZipFile.java Thu Feb 5 > 12:30:01 2009 > @@ -345,7 +345,15 @@ > > nameMap.put(ze.getName(), ze); > > - archive.skipBytes(extraLen); > + int lenToSkip = extraLen; > + while (lenToSkip > 0) { > + int skipped = archive.skipBytes(lenToSkip); > + if (skipped <= 0) { > + throw new RuntimeException("failed to skip extra data in" > + + " central directory"); > + } > + lenToSkip -= skipped; > + } > > byte[] comment = new byte[commentLen]; > archive.readFully(comment); > @@ -461,7 +469,15 @@ > int fileNameLen = ZipShort.getValue(b); > archive.readFully(b); > int extraFieldLen = ZipShort.getValue(b); > - archive.skipBytes(fileNameLen); > + int lenToSkip = fileNameLen; > + while (lenToSkip > 0) { > + int skipped = archive.skipBytes(lenToSkip); > + if (skipped <= 0) { > + throw new RuntimeException("failed to skip file name in" > + + " local file header"); > + } > + lenToSkip -= skipped; > + } > byte[] localExtraData = new byte[extraFieldLen]; > archive.readFully(localExtraData); > ze.setExtra(localExtraData); > > Modified: ant/core/trunk/src/main/org/apache/tools/zip/ZipLong.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/zip/ZipLong.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/zip/ZipLong.java (original) > +++ ant/core/trunk/src/main/org/apache/tools/zip/ZipLong.java Thu Feb 5 > 12:30:01 2009 > @@ -147,4 +147,13 @@ > public int hashCode() { > return (int) value; > } > + > + public Object clone() { > + try { > + return (ZipLong) super.clone(); > + } catch (CloneNotSupportedException cnfe) { > + // impossible > + throw new RuntimeException(cnfe); > + } > + } > } > > Modified: ant/core/trunk/src/main/org/apache/tools/zip/ZipShort.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/zip/ZipShort.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/main/org/apache/tools/zip/ZipShort.java (original) > +++ ant/core/trunk/src/main/org/apache/tools/zip/ZipShort.java Thu Feb 5 > 12:30:01 2009 > @@ -133,4 +133,13 @@ > public int hashCode() { > return value; > } > + > + public Object clone() { > + try { > + return (ZipShort) super.clone(); > + } catch (CloneNotSupportedException cnfe) { > + // impossible > + throw new RuntimeException(cnfe); > + } > + } > } > > Modified: > ant/core/trunk/src/tests/junit/org/apache/tools/zip/AsiExtraFieldTest.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/tests/junit/org/apache/tools/zip/AsiExtraFieldTest.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- > ant/core/trunk/src/tests/junit/org/apache/tools/zip/AsiExtraFieldTest.java > (original) > +++ > ant/core/trunk/src/tests/junit/org/apache/tools/zip/AsiExtraFieldTest.java > Thu Feb 5 12:30:01 2009 > @@ -138,4 +138,20 @@ > e.getMessage()); > } > } > + > + public void testClone() { > + AsiExtraField s1 = new AsiExtraField(); > + s1.setUserId(42); > + s1.setGroupId(12); > + s1.setLinkedFile("foo"); > + s1.setMode(0644); > + s1.setDirectory(true); > + AsiExtraField s2 = (AsiExtraField) s1.clone(); > + assertNotSame(s1, s2); > + assertEquals(s1.getUserId(), s2.getUserId()); > + assertEquals(s1.getGroupId(), s2.getGroupId()); > + assertEquals(s1.getLinkedFile(), s2.getLinkedFile()); > + assertEquals(s1.getMode(), s2.getMode()); > + assertEquals(s1.isDirectory(), s2.isDirectory()); > + } > } > > Modified: ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipLongTest.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipLongTest.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipLongTest.java > (original) > +++ ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipLongTest.java Thu > Feb 5 12:30:01 2009 > @@ -79,4 +79,11 @@ > assertEquals(0x00000000FFFFFFFFl, zl.getValue()); > } > > + public void testClone() { > + ZipLong s1 = new ZipLong(42); > + ZipLong s2 = (ZipLong) s1.clone(); > + assertNotSame(s1, s2); > + assertEquals(s1, s2); > + assertEquals(s1.getValue(), s2.getValue()); > + } > } > > Modified: > ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipShortTest.java > URL: > http://svn.apache.org/viewvc/ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipShortTest.java?rev=741089&r1=741088&r2=741089&view=diff > ============================================================================== > --- ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipShortTest.java > (original) > +++ ant/core/trunk/src/tests/junit/org/apache/tools/zip/ZipShortTest.java Thu > Feb 5 12:30:01 2009 > @@ -77,4 +77,11 @@ > assertEquals(0x0000FFFF, zs.getValue()); > } > > + public void testClone() { > + ZipShort s1 = new ZipShort(42); > + ZipShort s2 = (ZipShort) s1.clone(); > + assertNotSame(s1, s2); > + assertEquals(s1, s2); > + assertEquals(s1.getValue(), s2.getValue()); > + } > } > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org For additional commands, e-mail: dev-h...@ant.apache.org
