- removed comment
Include string null terminator in HDF5 attributes
Issue #1501
closed
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)
-
-
reporter
- changed status to open
- removed comment
-
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
-
reporter
- removed comment
The VisIt plugin also looks fine to me since it also constructs a c++ string from the null-terminated C string.
-
- 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 :-) ).
-
reporter
- changed status to resolved
- removed comment
Committed. Thanks for the quick review!
-
- edited description
- changed status to closed
- Log in to comment
- Assignee
- Type
- Priority
- Status
- closed
- Component
- Carpet
- Milestone
- –
- Version
- development version
- Votes
- 0
- Watchers
- 0
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. :-)