On 05/02/2009, Christian Grobmeier <grobme...@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-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
