commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [compress] SANDBOX-246 remaining Findbugs issues
Date Thu, 05 Feb 2009 18:12:43 GMT
On 05/02/2009, Christian Grobmeier <grobmeier@gmail.com> wrote:
> Hi,
>  thanks Stefan for running findbugs! I'll try to give answers as far as
>  I know till now.
>
>
>  > * JarArchiveEntry certificates and manifestAttributes is never written
>  >  to, so they are useless.
>  >
>  >  I'm unsure of the class' purpose and simply left things as they are,
>  >  assuming setters will be provided one day.
>
>
> It's planned that the *Entry classes provide additional information
>  for the archiver. For example see TarArchiveEntry, which gives you the
>  chance to add a username to the file. In the case of JarArchiveEntry
>  it's simply not finished and we need to extend this when the rest of
>  the work is more cool.
>
>
>  > * ZipOutputStream contains some proteted static final byte[]
>  >  "constants"
>  >
>  >  This means any subclass could modify them.  Findbugs suggests to
>  >  make the package private.  Ant couldn't do that because of backwards
>  >  incompatibility, but a sandbox component can.  Should we?
>
>
> I guess we can, but I think we should wait after the first release. At

But then you again have the problem of backwards compatibility.

I'd say make the byte[] arrays private if possible; there is likely a
better way to use the constants.

Leaving the constants exposed to mutation should be a last resort.

>  the moment I am copying Ant-codebase to here. If Ant is changing
>  forward I may have problems if we change such kind of stuff when
>  updating. Otherwise we will have development on the same code in two
>  projects and that may be diffcult.
>
>
>  > * ArchiveStreamFactory should do something when it fails to read
>  >  enough bytes for the signature, but what?
>  >
>  >  Given the original TODO comment, I stayed away from a decision.
>
>
> I have not thought about that. At first glance I guess it should throw
>  an ArchiverException.
>
>
>  > * CpioArchiveEntry#setMode first performs some work to check the mode
>  >  just passed in, creates an IllegalArgumentException if it is unknown
>  >  and then forgets to throw it.
>  >
>  >  If I change the code to actually throw the exception,
>  >  testCpioUnarchive fails.  Obviously the code supports more modes
>  >  than it thinks.
>
>
> The testcase is not the best I ever wrote. I can digg into this and
>  will improve all testcases. The CPIO stuff must be reviewed since it
>  came in from another project just before a while.
>
>
>  Christian
>
>
>
>  >  Remove the checks?
>
>
>
>  >
>  > 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
>
>

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


Mime
View raw message