DiamondLightSource / durin

BSD 3-Clause "New" or "Revised" License
2 stars 5 forks source link

Replace strcmp with strncmp in file.c #28

Closed graeme-winter closed 1 year ago

graeme-winter commented 1 year ago

For two reasons - we know the length of the string to compare with so strcmp is deadly, HDF5 strings for e.g. attributes may not be NULL terminated but most importantly strcmp on a not NULL terminated string returns false when it should be true if strncmp

Found in debugging with @dperl-dls

graeme-winter commented 1 year ago
Grey-Area durin :) [master] $ git grep strcmp .
src/file.c:     * terminated and extraneous bytes where being read by strcmp -  set the end
src/file.c:      if (strcmp("NXdata", nxclass) == 0) {
src/file.c:      } else if (strcmp("NXdetector", nxclass) == 0) {

Looks like the fix for this will be trivial. Turns out that is not the case. 🤔 much.

graeme-winter commented 1 year ago
nx class => NXentr / 7
nx class => NXdat / 6
nx class => NXinstrumen / 12
nx class => NXattenuato / 12
nx class => NXbea / 6
nx class => NXdetecto / 10
nx class => NXdetector_modul / 17
nx class => NXtransformation / 17
nx class => NXpositione / 12
nx class => NXsampl / 8
nx class => NXpositione / 12
nx class => NXpositione / 12
nx class => NXpositione / 12
nx class => NXpositione / 12
nx class => NXpositione / 12
nx class => NXpositione / 12
nx class => NXtransformation / 17
nx class => NXsourc / 8

quite possibly this is an issue in the implementation of

H5Aread(a_id, mt_id, buffer)

which I think may assume that the strings are NULL terminated, or allocated to be one larger than they need to be.

graeme-winter commented 1 year ago

strncmp is unhelpful because NXdetector and NXdetector_module so instead make sure that the strings are properly NULL terminated. In some cases this will end up with a double NULL at the end of the string but this is much safer