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