refactor v2 code

karlnicholas · karlnicholas · commit c78f37117ee7 · 2025-10-17T14:32:35.000-04:00
diff --git a/src/main/java/org/hdf5javalib/hdffile/dataobjects/messages/AttributeMessage.java b/src/main/java/org/hdf5javalib/hdffile/dataobjects/messages/AttributeMessage.java
@@ -89,48 +89,79 @@ public AttributeMessage(int version, HdfString name, DatatypeMessage datatypeMes
     /**
      * Parses an AttributeMessage from the provided data and file context.
      *
-     * @param flags       message flags
-     * @param data        the byte array containing the message data
-     * @param hdfDataFile the HDF5 file context for global heap and other resources
+     * @param messageFlags message flags
+     * @param data         the byte array containing the message data
+     * @param hdfDataFile  the HDF5 file context for global heap and other resources
      * @return a new AttributeMessage instance parsed from the data
      */
-    public static HdfMessage parseHeaderMessage(int flags, byte[] data, HdfDataFile hdfDataFile) throws InvocationTargetException, InstantiationException, IllegalAccessException, IOException {
+    public static HdfMessage parseHeaderMessage(int messageFlags, byte[] data, HdfDataFile hdfDataFile) throws InvocationTargetException, InstantiationException, IllegalAccessException, IOException {
         ByteBuffer buffer = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN);
         // Read the version (1 byte)
         int version = Byte.toUnsignedInt(buffer.get());
 
-        // Skip the reserved byte (1 byte, should be zero)
-        buffer.get();
-
-        // Read the sizes of name, datatype, and dataspace (2 bytes each)
-        int nameSize = Short.toUnsignedInt(buffer.getShort());
-        int datatypeSize = Short.toUnsignedInt(buffer.getShort());
-        int dataspaceSize = Short.toUnsignedInt(buffer.getShort());
-
-        // Read the name (variable size)
-        byte[] nameBytes = new byte[nameSize];
-        buffer.get(nameBytes);
-        BitSet bitSet = StringDatatype.createClassBitField(StringDatatype.PaddingType.NULL_TERMINATE, StringDatatype.CharacterSet.ASCII);
-        HdfString name = new HdfString(nameBytes, new StringDatatype(StringDatatype.createClassAndVersion(), bitSet, nameSize, hdfDataFile));
-        // get padding bytes
-        int padding = (8 - (nameSize % 8)) % 8;
-        byte[] paddingBytes = new byte[padding];
-        buffer.get(paddingBytes);
-
-        byte[] dtBytes = new byte[datatypeSize];
-        buffer.get(dtBytes);
-        // get padding bytes
-        padding = (8 - (datatypeSize % 8)) % 8;
-        paddingBytes = new byte[padding];
-        buffer.get(paddingBytes);
-
-        byte[] dsBytes = new byte[dataspaceSize];
-        buffer.get(dsBytes);
-        // get padding bytes
-        padding = (8 - (dataspaceSize % 8)) % 8;
-        paddingBytes = new byte[padding];
-        buffer.get(paddingBytes);
+        int nameSize;
+        int datatypeSize;
+        int dataspaceSize;
+        HdfString name;
+
+        byte[] dtBytes;
+        byte[] dsBytes;
+
+        if (version == 1) {
+            // Version 1 requires 8-byte padding between fields.
+            buffer.get(); // Skip reserved byte
+            nameSize = Short.toUnsignedInt(buffer.getShort());
+            datatypeSize = Short.toUnsignedInt(buffer.getShort());
+            dataspaceSize = Short.toUnsignedInt(buffer.getShort());
+
+            byte[] nameBytes = new byte[nameSize];
+            buffer.get(nameBytes);
+            BitSet bitSet = StringDatatype.createClassBitField(StringDatatype.PaddingType.NULL_TERMINATE, StringDatatype.CharacterSet.ASCII);
+            name = new HdfString(nameBytes, new StringDatatype(StringDatatype.createClassAndVersion(), bitSet, nameSize, hdfDataFile));
+
+            // Apply 8-byte alignment padding after the name
+            buffer.position((buffer.position() + 7) & ~7);
+
+            dtBytes = new byte[datatypeSize];
+            buffer.get(dtBytes);
+            // Apply 8-byte alignment padding after the datatype
+            buffer.position((buffer.position() + 7) & ~7);
+
+            dsBytes = new byte[dataspaceSize];
+            buffer.get(dsBytes);
+            // Apply 8-byte alignment padding after the dataspace
+            buffer.position((buffer.position() + 7) & ~7);
+
+        } else if (version == 2 || version == 3) {
+            // Versions 2 & 3 have NO PADDING between fields.
+            buffer.get(); // Read flags (for v3) or reserved byte (for v2)
+            nameSize = Short.toUnsignedInt(buffer.getShort());
+            datatypeSize = Short.toUnsignedInt(buffer.getShort());
+            dataspaceSize = Short.toUnsignedInt(buffer.getShort());
+
+            StringDatatype.CharacterSet characterSet = StringDatatype.CharacterSet.ASCII;
+            if (version == 3) {
+                byte charSetByte = buffer.get();
+                characterSet = (charSetByte == 0) ? StringDatatype.CharacterSet.ASCII : StringDatatype.CharacterSet.UTF8;
+            }
+
+            byte[] nameBytes = new byte[nameSize];
+            buffer.get(nameBytes);
+            BitSet bitSet = StringDatatype.createClassBitField(StringDatatype.PaddingType.NULL_TERMINATE, characterSet);
+            name = new HdfString(nameBytes, new StringDatatype(StringDatatype.createClassAndVersion(), bitSet, nameSize, hdfDataFile));
+
+            // Read fields sequentially with no padding
+            dtBytes = new byte[datatypeSize];
+            buffer.get(dtBytes);
+
+            dsBytes = new byte[dataspaceSize];
+            buffer.get(dsBytes);
 
+        } else {
+            throw new IOException("Unsupported AttributeMessage version: " + version);
+        }
+
+        // --- (This section is restored to your original code) ---
         HdfMessage hdfDataObjectHeaderDt = parseHeaderMessage(MessageType.DATATYPE_MESSAGE, (byte) 0, dtBytes, hdfDataFile);
         DatatypeMessage dt = (DatatypeMessage) hdfDataObjectHeaderDt;
         HdfMessage hdfDataObjectHeaderDs = parseHeaderMessage(MessageType.DATASPACE_MESSAGE, (byte) 0, dsBytes, hdfDataFile);
@@ -146,7 +177,7 @@ public static HdfMessage parseHeaderMessage(int flags, byte[] data, HdfDataFile
             byte[] dataBytes = new byte[dtDataSize];
             buffer.get(dataBytes);
             HdfData scalarValue = dt.getHdfDatatype().getInstance(HdfData.class, dataBytes);
-            return new AttributeMessage(version, name, dt, ds, HdfDataHolder.ofScalar(scalarValue), flags, data.length);
+            return new AttributeMessage(version, name, dt, ds, HdfDataHolder.ofScalar(scalarValue), messageFlags, data.length);
         }
 
         // Case 2: Array data (dimensionality is 1 or more)
@@ -160,15 +191,12 @@ public static HdfMessage parseHeaderMessage(int flags, byte[] data, HdfDataFile
         dimensions = Arrays.copyOfRange(dimensions, 0, count);
 
         // Step 1: Create the n-dimensional array dynamically.
-        // Array.newInstance() is the key. It can create an array of any type with any dimensions.
-        // Example: Array.newInstance(HdfData.class, 2, 3) creates a HdfData[2][3]
         Object multiDimArray = Array.newInstance(HdfData.class, dimensions);
 
         // Step 2: Populate the array recursively from the flat buffer.
         populateArray(multiDimArray, dimensions, 0, buffer, dt.getHdfDatatype(), dtDataSize);
 
-        return new AttributeMessage(version, name, dt, ds, HdfDataHolder.ofArray(multiDimArray, dimensions), flags, data.length);
-
+        return new AttributeMessage(version, name, dt, ds, HdfDataHolder.ofArray(multiDimArray, dimensions), messageFlags, data.length);
     }
 
     /**
diff --git a/src/main/java/org/hdf5javalib/hdffile/dataobjects/messages/DataspaceMessage.java b/src/main/java/org/hdf5javalib/hdffile/dataobjects/messages/DataspaceMessage.java
@@ -94,28 +94,46 @@ public DataspaceMessage(
     /**
      * Parses a DataspaceMessage from the provided data and file context.
      *
-     * @param flags       message flags
-     * @param data        the byte array containing the message data
-     * @param hdfDataFile the HDF5 file context for datatype resources
+     * @param messageFlags message flags
+     * @param data         the byte array containing the message data
+     * @param hdfDataFile  the HDF5 file context for datatype resources
      * @return a new DataspaceMessage instance parsed from the data
      */
-    public static HdfMessage parseHeaderMessage(int flags, byte[] data, HdfDataFile hdfDataFile) throws IOException, InvocationTargetException, InstantiationException, IllegalAccessException {
+    public static HdfMessage parseHeaderMessage(int messageFlags, byte[] data, HdfDataFile hdfDataFile) throws IOException, InvocationTargetException, InstantiationException, IllegalAccessException {
         ByteBuffer buffer = ByteBuffer.wrap(data).order(ByteOrder.LITTLE_ENDIAN);
 
         int version = Byte.toUnsignedInt(buffer.get());
-        int dimensionality = Byte.toUnsignedInt(buffer.get());
-        int flagByte = Byte.toUnsignedInt(buffer.get());
-        BitSet flagSet = BitSet.valueOf(new byte[]{(byte) flagByte});
+        int dimensionality;
+        BitSet flagSet;
+
         if (version == 1) {
+            dimensionality = Byte.toUnsignedInt(buffer.get());
+            byte flagByte = buffer.get();
+            flagSet = BitSet.valueOf(new byte[]{flagByte});
+            // Version 1 has 5 reserved bytes that must be skipped.
             buffer.position(buffer.position() + DATASPACE_MESSAGE_RESERVED_1);
-        } else {
+
+        } else if (version == 2) {
+            dimensionality = Byte.toUnsignedInt(buffer.get());
+            byte flagByte = buffer.get();
+            flagSet = BitSet.valueOf(new byte[]{flagByte});
+            // Version 2 has a 'Type' field here. We must read it to advance the buffer,
+            // even if we don't use the 'type' value directly.
             int type = Byte.toUnsignedInt(buffer.get());
+
+        } else {
+            throw new IOException("Unsupported DataspaceMessage version: " + version);
         }
+
+        // --- Common logic for reading dimensions ---
+        // This part now works correctly because the buffer is positioned properly for any supported version.
         HdfFixedPoint[] dimensions = new HdfFixedPoint[dimensionality];
         for (int i = 0; i < dimensionality; i++) {
             dimensions[i] = HdfReadUtils.readHdfFixedPointFromBuffer(hdfDataFile.getSuperblock().getFixedPointDatatypeForLength(), buffer);
         }
 
+        // The spec states bit 0 of the flags indicates if max dimensions are present.
+        // Assuming your DataspaceFlag enum handles this correctly.
         boolean hasMaxDimensions = flagSet.get(DataspaceFlag.MAX_DIMENSIONS_PRESENT.getBitIndex());
         HdfFixedPoint[] maxDimensions = null;
         if (hasMaxDimensions) {
@@ -125,9 +143,7 @@ public static HdfMessage parseHeaderMessage(int flags, byte[] data, HdfDataFile
             }
         }
 
-        return new DataspaceMessage(version, dimensionality, flagSet, dimensions, maxDimensions, hasMaxDimensions, flags, data.length);
-
-
+        return new DataspaceMessage(version, dimensionality, flagSet, dimensions, maxDimensions, hasMaxDimensions, messageFlags, data.length);
     }
 
     /**
