areaDetector / ADCore

The home of the core components of the EPICS areaDetector software. It includes base classes for drivers and code for all of the standard plugins.
https://areadetector.github.io/master/index.html
Other
20 stars 65 forks source link

Include trailing null in size passed to H5Tset_size() #461

Closed MarkRivers closed 3 years ago

MarkRivers commented 3 years ago

The size passed to H5Tset_size() is supposed to include the trailing null character. The current code is not doing that. This results in error messages from the HDF5 library if a zero length string is passed. The file structure is also not correct, the string data should include the trailing null character.

This fixes issue #460.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 38.9% when pulling bfa31513dddf903d13f1e56d7f870f06eacc8ffd on fix_hdf5_string_size into 9321f2a55120a41ce168d2021bcc34f3c6f09198 on master.

MarkRivers commented 3 years ago

I have verified that with this PR:

This is the output of h5dump with a non-zero length constant string dataset written before the fix:

(base) corvette:~/scratch>h5dump -p -d /detector/det_name hdf5_string_test_001.h5
HDF5 "hdf5_string_test_001.h5" {
DATASET "/detector/det_name" {
   DATATYPE  H5T_STRING {
      STRSIZE 18;
      STRPAD H5T_STR_NULLTERM;
      CSET H5T_CSET_ASCII;
      CTYPE H5T_C_S1;
   }
   DATASPACE  SIMPLE { ( 1 ) / ( 1 ) }
   STORAGE_LAYOUT {
      CONTIGUOUS
      SIZE 18
      OFFSET 2968
   }
   FILTERS {
      NONE
   }
   FILLVALUE {
      FILL_TIME H5D_FILL_TIME_IFSET
      VALUE  H5D_FILL_VALUE_DEFAULT
   }
   ALLOCATION_TIME {
      H5D_ALLOC_TIME_LATE
   }
   DATA {
   (0): "Simulated detector"
   }
}
}

This is the output of h5dump on that dataset after the fix:

(base) corvette:~/scratch>h5dump -p -d /detector/det_name hdf5_string_test_012.h5
HDF5 "hdf5_string_test_012.h5" {
DATASET "/detector/det_name" {
   DATATYPE  H5T_STRING {
      STRSIZE 19;
      STRPAD H5T_STR_NULLTERM;
      CSET H5T_CSET_ASCII;
      CTYPE H5T_C_S1;
   }
   DATASPACE  SIMPLE { ( 1 ) / ( 1 ) }
   STORAGE_LAYOUT {
      CONTIGUOUS
      SIZE 19
      OFFSET 2968
   }
   FILTERS {
      NONE
   }
   FILLVALUE {
      FILL_TIME H5D_FILL_TIME_IFSET
      VALUE  H5D_FILL_VALUE_DEFAULT
   }
   ALLOCATION_TIME {
      H5D_ALLOC_TIME_LATE
   }
   DATA {
   (0): "Simulated detector"
   }
}
}

In both cases the string is "Simulated detector" which is 18 bytes long. Before the fix the SIZE reported by h5dump is 18, after the fix it is 19 because it includes the trailing null. But the DATA printed is the same.