Include string null terminator in HDF5 attributes

Create issue
Issue #1501 closed
Barry Wardell created an issue

The HDF5 documentation http://www.hdfgroup.org/HDF5/doc/RM/RM_H5T.html#Datatype-SetSize states that the size of a string should include space for a null terminator. Up to now, the "name" attribute in HDF5 output files was stored as a string excluding the null terminator. This meant that when the attributed was read back, it could not be treated as a standard C string since it was missing the null terminator.

The attached patch changes the behaviour so that the null terminator is included.

Keyword:

Comments (7)

  1. Roland Haas
    • removed comment

    This requires more changes. Currently our code is written to re-add the NUL byte when the attribute is read in again (line 1248 of CarpetIOHDF5/src/Input.cc since char vector a guaranteed to be initialized to zero). Similarly in the slicers etc. So in order to not duplicate the NUL byte (almost certainly harmless and might well be stripped accidentally by an strlen) we might need to adjust the reader logic to strip duplicate NUL bytes at the end.

    So currently I think our code actually is bug free, but does not adhere to the published HDF5 manual. I think it would be good to include the NUL byte since it will make using standard HDF5 tools easier.

    So: the code looks fine, please also check the reader in Input.cc and the various HDF5 tools in CarpetIOHDF5/src/utils and ExternalLibraries/HDF5/src as well as (ideally) the Carpet VisIt plugin. :-)

  2. Barry Wardell reporter
    • removed comment

    I checked Input.cc and it already avoids duplicated NUL since it constructs a C++ string from a C string and works from then on with the C++ string.

    The only place I could find a potential problem (I didn't check the VisIt plugin) was in CarpetIOHDF5/src/util/hdf5_slicer.cc. The updated patch also fixes that file so that it works in the same way as Input.cc

  3. Barry Wardell reporter
    • removed comment

    The VisIt plugin also looks fine to me since it also constructs a c++ string from the null-terminated C string.

  4. Roland Haas
    • changed status to open
    • removed comment

    Please apply. Do you have commit rights to Carpet? Otherwise I can do the commit (I can even use your name :-) ).

  5. Log in to comment