You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2009/03/30 17:07:54 UTC

svn commit: r760000 - in /commons/proper/compress/trunk/src: main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java test/java/org/apache/commons/compress/archivers/ArTestCase.java

Author: bodewig
Date: Mon Mar 30 15:07:54 2009
New Revision: 760000

URL: http://svn.apache.org/viewvc?rev=760000&view=rev
Log:
make closeArchiveEntry a NOP if called repeatedly.  This makes ArTestCase fail for all platforms consistently 8-)

Modified:
    commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
    commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java

Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java?rev=760000&r1=759999&r2=760000&view=diff
==============================================================================
--- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java (original)
+++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java Mon Mar 30 15:07:54 2009
@@ -35,6 +35,7 @@
     private long archiveOffset = 0;
     private long entryOffset = 0;
     private ArArchiveEntry prevEntry;
+    private boolean haveUnclosedEntry = true;
 
     public ArArchiveOutputStream( final OutputStream pOut ) {
         this.out = pOut;
@@ -47,10 +48,11 @@
     }
 
     public void closeArchiveEntry() throws IOException {
-        if ((entryOffset % 2) != 0) {
+        if (prevEntry != null && haveUnclosedEntry && (entryOffset % 2) != 0) {
             out.write('\n'); // Pad byte
             archiveOffset++;
         }
+        haveUnclosedEntry = false;
     }
 
     public void putArchiveEntry( final ArchiveEntry pEntry ) throws IOException {
@@ -70,6 +72,7 @@
         archiveOffset += writeEntryHeader(pArEntry);
 
         entryOffset = 0;
+        haveUnclosedEntry = true;
     }
 
     private long fill( final long pOffset, final long pNewOffset, final char pFill ) throws IOException { 

Modified: commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java
URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java?rev=760000&r1=759999&r2=760000&view=diff
==============================================================================
--- commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java (original)
+++ commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java Mon Mar 30 15:07:54 2009
@@ -89,10 +89,10 @@
     public void testArDelete() throws Exception {
         final File output = new File(dir, "bla.ar");
 
+        final File file1 = getFile("test1.xml");
+        final File file2 = getFile("test2.xml");
         {
             // create
-            final File file1 = getFile("test1.xml");
-            final File file2 = getFile("test2.xml");
 
             final OutputStream out = new FileOutputStream(output);
             final ArchiveOutputStream os = new ArchiveStreamFactory().createArchiveOutputStream("ar", out);
@@ -107,7 +107,10 @@
             out.close();
         }
 
-        assertEquals(282, output.length());
+        assertEquals(8
+                     + 60 + file1.length() + (file1.length() % 2)
+                     + 60 + file2.length() + (file2.length() % 2),
+                     output.length());
 
         final File output2 = new File(dir, "bla2.ar");
 



Re: svn commit: r760000 - in /commons/proper/compress/trunk/src: main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java test/java/org/apache/commons/compress/archivers/ArTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-30, sebb <se...@gmail.com> wrote:

> On 30/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
>> On 2009-03-30, sebb <se...@gmail.com> wrote:

>>> Might it not be better to throw an Exception if methods are called out
>>> of sequence?


>> We are pretty inconsistent here.  In general I agree it would be
>>  better to throw an exception, but we seem to swallow other errors in
>>  other classes.

> In that case, why don't we fix the other classes?

I took the fact that we are inconsistent as a hint that we don't know
what we want to do.  It could as well mean that we've just pulled
together implementations with different ideas from three sources and
haven't thought about making them consistent so far 8-)

> If we document the required behaviour (e.g. in the abstract i/o
> classes), then we can work towards making the code consistent.

No problem with that.

BTW, please add yourself as a comitter to the POM, you've by now
contributed more than enough to be listed there.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r760000 - in /commons/proper/compress/trunk/src: main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java test/java/org/apache/commons/compress/archivers/ArTestCase.java

Posted by sebb <se...@gmail.com>.
On 30/03/2009, Stefan Bodewig <bo...@apache.org> wrote:
> On 2009-03-30, sebb <se...@gmail.com> wrote:
>
>  > Might it not be better to throw an Exception if methods are called out
>  > of sequence?
>
>
> We are pretty inconsistent here.  In general I agree it would be
>  better to throw an exception, but we seem to swallow other errors in
>  other classes.

In that case, why don't we fix the other classes?

If we document the required behaviour (e.g. in the abstract i/o
classes), then we can work towards making the code consistent.

>  Stefan
>
>  ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r760000 - in /commons/proper/compress/trunk/src: main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java test/java/org/apache/commons/compress/archivers/ArTestCase.java

Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-03-30, sebb <se...@gmail.com> wrote:

> Might it not be better to throw an Exception if methods are called out
> of sequence?

We are pretty inconsistent here.  In general I agree it would be
better to throw an exception, but we seem to swallow other errors in
other classes.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r760000 - in /commons/proper/compress/trunk/src: main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java test/java/org/apache/commons/compress/archivers/ArTestCase.java

Posted by sebb <se...@gmail.com>.
On 30/03/2009, bodewig@apache.org <bo...@apache.org> wrote:
> Author: bodewig
>  Date: Mon Mar 30 15:07:54 2009
>  New Revision: 760000
>
>  URL: http://svn.apache.org/viewvc?rev=760000&view=rev
>  Log:
>  make closeArchiveEntry a NOP if called repeatedly.  This makes ArTestCase fail for all platforms consistently 8-)

Might it not be better to throw an Exception if methods are called out
of sequence?
The class would have to check before calling closeAE in close(), but
it seems to me that it would help trap user coding errors.

Allowing multiple calls to close() is a different case, as that makes
it easier for code to handle tidying up after exceptions etc.

>  Modified:
>     commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
>     commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java
>
>  Modified: commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
>  URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java?rev=760000&r1=759999&r2=760000&view=diff
>  ==============================================================================
>  --- commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java (original)
>  +++ commons/proper/compress/trunk/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java Mon Mar 30 15:07:54 2009
>  @@ -35,6 +35,7 @@
>      private long archiveOffset = 0;
>      private long entryOffset = 0;
>      private ArArchiveEntry prevEntry;
>  +    private boolean haveUnclosedEntry = true;
>
>      public ArArchiveOutputStream( final OutputStream pOut ) {
>          this.out = pOut;
>  @@ -47,10 +48,11 @@
>      }
>
>      public void closeArchiveEntry() throws IOException {
>  -        if ((entryOffset % 2) != 0) {
>  +        if (prevEntry != null && haveUnclosedEntry && (entryOffset % 2) != 0) {
>              out.write('\n'); // Pad byte
>              archiveOffset++;
>          }
>  +        haveUnclosedEntry = false;
>      }
>
>      public void putArchiveEntry( final ArchiveEntry pEntry ) throws IOException {
>  @@ -70,6 +72,7 @@
>          archiveOffset += writeEntryHeader(pArEntry);
>
>          entryOffset = 0;
>  +        haveUnclosedEntry = true;
>      }
>
>      private long fill( final long pOffset, final long pNewOffset, final char pFill ) throws IOException {
>
>  Modified: commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java
>  URL: http://svn.apache.org/viewvc/commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java?rev=760000&r1=759999&r2=760000&view=diff
>  ==============================================================================
>  --- commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java (original)
>  +++ commons/proper/compress/trunk/src/test/java/org/apache/commons/compress/archivers/ArTestCase.java Mon Mar 30 15:07:54 2009
>  @@ -89,10 +89,10 @@
>      public void testArDelete() throws Exception {
>          final File output = new File(dir, "bla.ar");
>
>  +        final File file1 = getFile("test1.xml");
>  +        final File file2 = getFile("test2.xml");
>          {
>              // create
>  -            final File file1 = getFile("test1.xml");
>  -            final File file2 = getFile("test2.xml");
>
>              final OutputStream out = new FileOutputStream(output);
>              final ArchiveOutputStream os = new ArchiveStreamFactory().createArchiveOutputStream("ar", out);
>  @@ -107,7 +107,10 @@
>              out.close();
>          }
>
>  -        assertEquals(282, output.length());
>  +        assertEquals(8
>  +                     + 60 + file1.length() + (file1.length() % 2)
>  +                     + 60 + file2.length() + (file2.length() % 2),
>  +                     output.length());
>
>          final File output2 = new File(dir, "bla2.ar");
>
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org