Review request: Install module content into zip files

mark.reinhold at oracle.com mark.reinhold at oracle.com
Fri May 21 11:10:09 PDT 2010


> Date: Fri, 21 May 2010 16:01:39 +0200
> From: dalibor.topic at sun.com

> Nice, I like the extensions to Files, in particular, and the cleanup in
> ModuleFileFormat. Small comments:
> 
> ModuleFileFormat.java:
> 
>  632         public void readUncompressedFile(DataInputStream in,
>  633                                          ModuleFile.SectionType type,
>  634                                          int csize)
>  635             throws IOException
>  636         {
>  637             assert type != ModuleFile.SectionType.MODULE_INFO;
> 
> -> this assert is gone in the patch. I'm not sure why, as the only
> uncompressed format atm is the module info file.

The previous version of this method included the assert but also some
later logic to handle the case of type == MODULE_INFO.  While refactoring
I removed that later logic and also the assert.  I'll restore the assert.

>  730         private void unpack200gzip(DataInputStream in) throws
> IOException {
> [..]
> 735             unpacker.unpack(gis, contentStream());
> 
> doesn't close the content stream.

It doesn't need to, and in fact it shouldn't since resources can follow
in the module file.  The content stream is closed by the MFF.close()
method.

> Files.java:
> 
> Nice. A few things I could use would be:
> 
> 1) a public void copy(InputStream, OutputStream) would be useful for
> replacing the various bits in ModuleFileFormat doing the same thing.
> 
> 2) It would be nice if the copy methods returned the number of bytes
> copied. Same for storeTree -> that would make it easy to use those
> methods during module file writing to get the usize right.

Feel free to revise/enhance Files.java as needed.

Thanks for the review.

- Mark



More information about the jigsaw-dev mailing list