ModuleFileFormat.java patches

Mark Reinhold mr at sun.com
Thu Feb 25 13:58:50 PST 2010


Attached below is the patch I've been using on top of the initial
ModuleFileFormat patch you sent me a few weeks ago.  Now that your
initial commit is done, could you please look at integrating these
changes, adapting them as needed to the latest version?

Summary:

  - Split readModule() into readStart(), which just returns the
    module-info bytes, and readRest(), which reads the rest of the
    stream and writes content into files

  - Added a close() method

  - Changed the shape of the output tree so that it matches what
    SimpleLibrary expects:

        $DST/info             // module-info.class
            /classes/...
            /resources/...
            /lib/...          // Native libraries
            /bin/...          // Native commands
            /etc/...          // Configuration files

  - Fixed the markNativeCodeExecutable method to work correctly

Another issue I noticed in your initial patch, but didn't fix, is the
byte-by-byte copyStream method.  That's practically guaranteed to be a
performance problem down the road.  It should really use an 8KB buffer.

Thanks,
- Mark

----
# HG changeset patch
# Date 1266874955 28800

Module-file patches

diff --git a/src/share/classes/org/openjdk/jigsaw/ModuleFileFormat.java b/src/share/classes/org/openjdk/jigsaw/ModuleFileFormat.java
--- a/src/share/classes/org/openjdk/jigsaw/ModuleFileFormat.java
+++ b/src/share/classes/org/openjdk/jigsaw/ModuleFileFormat.java
@@ -384,14 +384,11 @@
 	    }
 	}
 
-	public Reader(DataInputStream stream, File destination) {
+	public Reader(DataInputStream stream) {
 	    hashtype = ModuleFile.HashType.SHA256;
 	    this.stream = stream;
-	    this.destination = destination;
 	}
 
-	
-
 	private void checkHashMatch(byte [] expected, byte [] computed) 
 	    throws IOException {
 	    if (!Arrays.equals(expected, computed))
@@ -401,22 +398,59 @@
 				      + hashHexString(computed));
 	}
 
-	public void readModule() throws IOException {
-	    ModuleFileHeader header = ModuleFileHeader.read(stream);
-	    // System.out.println(header.toString());
-	    MessageDigest md = getHashInstance(hashtype);
-	    DigestInputStream dis = new DigestInputStream(stream, md);
-	    DataInputStream in = new DataInputStream(dis);
+        private ModuleFileHeader fileHeader = null;
+        private MessageDigest fileDigest = null;
+        private DataInputStream fileIn = null;
+        private byte[] moduleInfoBytes = null;
 
-	    for (short sections = header.getSections(); 
-		 sections > 0;
-		 sections --) 
-		readSection(in);
+        // Reads the MODULE_INFO section, but does not write any files
+        //
+	public byte[] readStart() throws IOException {
+            try {
+                fileHeader = ModuleFileHeader.read(stream);
+                // System.out.println(fileHeader.toString());
+                fileDigest = getHashInstance(hashtype);
+                DigestInputStream dis = new DigestInputStream(stream, fileDigest);
+                fileIn = new DataInputStream(dis);
+                if (fileHeader.getSections() < 1)
+                    throw new IOException("A module file must have"
+                                          + " at least one section");
+                if (readSection(fileIn) != ModuleFile.SectionType.MODULE_INFO)
+                    throw new IOException("First module-file section"
+                                          + " is not MODULE_INFO");
+                assert moduleInfoBytes != null;
+                return moduleInfoBytes;
+            } catch (IOException x) {
+                close();
+                throw x;
+            }
+        }
 
-	    checkHashMatch(header.getHash(), md.digest());
+        public void readRest(File dst) throws IOException {
+            destination = dst;
+            try {
+                Files.store(moduleInfoBytes, computeRealPath("info"));
+                for (int ns = fileHeader.getSections() - 1; ns > 0; ns--)
+                    readSection(fileIn);
+                checkHashMatch(fileHeader.getHash(), fileDigest.digest());
+            } finally {
+                close();
+            }
 	}
 
-	private void readSection(DataInputStream stream)
+        public void close() throws IOException {
+            if (fileIn != null) {
+                fileIn.close();
+                fileIn = null;
+            }
+        }
+
+        public void readModule(File dst) throws IOException {
+            readStart();
+            readRest(dst);
+        }
+
+	private ModuleFile.SectionType readSection(DataInputStream stream)
 	    throws IOException {
 
 	    SectionHeader header = SectionHeader.read(stream);
@@ -436,6 +470,8 @@
 		readFile(in, compressor, type, csize);
 
 	    checkHashMatch(header.getHash(), md.digest());
+
+            return header.getType();
 	}
 
         public void readFile(DataInputStream in, 
@@ -446,9 +482,10 @@
 
             switch (compressor) {
 	    case NONE:
-		boolean readheader = 
-		    ModuleFile.SectionType.MODULE_INFO == type ? false : true;
-		readUncompressedFile(in, readheader, csize);
+                if (type == ModuleFile.SectionType.MODULE_INFO)
+                    moduleInfoBytes = readModuleInfo(in, csize);
+                else
+                    readUncompressedFile(in, type, csize);
 		break;
 	    case GZIP:
 		readGZIPCompressedFile(in, type);
@@ -472,9 +509,7 @@
 
 	    SubSectionFileHeader header = SubSectionFileHeader.read(in);
 	    int csize = header.getCSize();
-	    String filename = 
-		type.toString() + File.separator + header.getPath();
-	    File path = computeRealPath(filename);
+	    File path = computeRealPath(type, header.getPath());
 
             // Splice off the compressed file from input stream
 	    ByteArrayOutputStream baos = new ByteArrayOutputStream();
@@ -490,21 +525,18 @@
             out.close();
 	    gin.close();
             
-	    markNativeCodeExecutable(path);
+	    markNativeCodeExecutable(type, path);
 	}
 
-        public void readUncompressedFile(DataInputStream in, boolean readheader, 
-					 int csize) throws IOException {
-	    File realpath;
-
-	    // module-info.class has no header
-	    if (readheader) {
-		SubSectionFileHeader header = SubSectionFileHeader.read(in);
-		csize = header.getCSize();
-		realpath = computeRealPath(header.getPath());
-	    }
-	    else 
-		realpath = computeRealPath("module-info.class");
+        public void readUncompressedFile(DataInputStream in,
+                                         ModuleFile.SectionType type,
+					 int csize)
+            throws IOException
+        {
+            assert type != ModuleFile.SectionType.MODULE_INFO;
+            SubSectionFileHeader header = SubSectionFileHeader.read(in);
+            csize = header.getCSize();
+            File realpath = computeRealPath(type, header.getPath());
 
             // Create the file 
 	    File parent = realpath.getParentFile();
@@ -512,14 +544,35 @@
 		Files.mkdirs(parent, realpath.getName());
 
             // Write the file
-            OutputStream os = new FileOutputStream(realpath);
-            BufferedOutputStream out = new BufferedOutputStream(os);
-            copyStream(new CountingInputStream(in, csize), out, csize);
-            out.close();
-            
-	    markNativeCodeExecutable(realpath);
+            OutputStream out = new FileOutputStream(realpath);
+            CountingInputStream cin = new CountingInputStream(in, csize);
+            ByteArrayOutputStream bout = null;
+            if (type == ModuleFile.SectionType.MODULE_INFO)
+                bout = new ByteArrayOutputStream();
+            byte[] buf = new byte[8192];
+            int n;
+            while ((n = cin.read(buf)) >= 0) {
+                out.write(buf, 0, n);
+                if (bout != null)
+                    bout.write(buf, 0, n);
+            }
+            if (type == ModuleFile.SectionType.MODULE_INFO)
+                moduleInfoBytes = bout.toByteArray();
+
+	    markNativeCodeExecutable(type, realpath);
         }
 
+        public byte[] readModuleInfo(DataInputStream in, int csize)
+            throws IOException
+        {
+            CountingInputStream cin = new CountingInputStream(in, csize);
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            byte[] buf = new byte[8192];
+            int n;
+            while ((n = cin.read(buf)) >= 0)
+                out.write(buf, 0, n);
+            return out.toByteArray();
+        }
 
 	private File computeRealPath(String storedpath) throws IOException {
             String convertedpath = storedpath.replace('/', File.separatorChar);
@@ -536,12 +589,42 @@
 	    return path;
 	}
 
-	private static void markNativeCodeExecutable(File file) {
-	    // Set executable bit on executable files
-	    String path = file.toString();
-            if (path.endsWith(".so") || path.endsWith(".dll")
-                || path.endsWith(".exe"))
-               file.setExecutable(true);
+	private File computeRealPath(ModuleFile.SectionType type,
+                                     String storedpath)
+            throws IOException
+        {
+            String dir = null;
+            switch (type) {
+            case CLASSES:
+                dir = "classes";
+                break;
+            case RESOURCES:
+                dir = "resources";
+                break;
+            case NATIVE_LIBS:
+                dir = "lib";
+                break;
+            case NATIVE_CMDS:
+                dir = "bin";
+                break;
+            case CONFIG:
+                dir = "etc";
+                break;
+            default:
+                throw new AssertionError(type);
+            }
+            return computeRealPath(dir + File.separatorChar + storedpath);
+        }
+
+	private static void markNativeCodeExecutable(ModuleFile.SectionType type,
+                                                     File file)
+        {
+            if (type == ModuleFile.SectionType.NATIVE_CMDS
+                || (type == ModuleFile.SectionType.NATIVE_LIBS
+                    && System.getProperty("os.name").startsWith("Windows")))
+            {
+                file.setExecutable(true);
+            }
 	}
 
 	private JarInputStream unpack200gzip(DataInputStream in) 
@@ -567,8 +650,8 @@
 	    for (JarEntry entry = jin.getNextJarEntry();
 		 entry != null;
 		 entry = jin.getNextJarEntry()) {
-		File path = computeRealPath(entry.getName());
-		
+		File path = computeRealPath(ModuleFile.SectionType.CLASSES,
+                                            entry.getName());
 		FileOutputStream file = new FileOutputStream(path);
 		copyStream(jin, file);
 		file.close();
diff --git a/src/share/classes/org/openjdk/jigsaw/cli/Librarian.java b/src/share/classes/org/openjdk/jigsaw/cli/Librarian.java
--- a/src/share/classes/org/openjdk/jigsaw/cli/Librarian.java
+++ b/src/share/classes/org/openjdk/jigsaw/cli/Librarian.java
@@ -137,8 +137,8 @@
 		    FileInputStream fis = new FileInputStream(module);
 		    DataInputStream dis = new DataInputStream(fis);
 		    ModuleFileFormat.Reader reader = 
-			new ModuleFileFormat.Reader(dis, classes);
-		    reader.readModule();
+			new ModuleFileFormat.Reader(dis);
+		    reader.readModule(classes);
 		    String name = module.getName();
 		    name = name.substring(0, name.lastIndexOf(".jxr"));
 		    Files.copyTree(classes, new File(name));



More information about the jigsaw-dev mailing list