ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Reilly <peter.kitt.rei...@gmail.com>
Subject Re: svn commit: r741089 - in /ant/core/trunk/src: main/org/apache/tools/bzip2/ main/org/apache/tools/tar/ main/org/apache/tools/zip/ tests/junit/org/apache/tools/zip/
Date Thu, 05 Feb 2009 13:39:20 GMT
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,  <bodewig@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-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Mime
View raw message