Skip to content

FstatToplist: simplify string ops in write_hs_checkpoint()

David Keitel requested to merge david-keitel/lalsuite:FstatToplist-string-ops into master

Description

There's an error message from gcc 9.3.0 in this experimental conda job, which may well crop up in more mainstream builds later as well, so let's better fix it now:

make[8]: Entering directory '/builds/duncanmmacleod/lalsuite/lalapps/src/pulsar/Hough'
  CC       HoughValidate.o
  CC       PeakSelect.o
  CC       DriveHoughMulti.o
  CC       FstatToplist.o
In file included from /opt/conda/envs/lalsuite-platform-tests/x86_64-conda-linux-gnu/sysroot/usr/include/string.h:642,
                 from /builds/duncanmmacleod/lalsuite/lal/include/lal/LALStatusMacros.h:33,
                 from /builds/duncanmmacleod/lalsuite/lal/include/lal/LALStdlib.h:58,
                 from /builds/duncanmmacleod/lalsuite/lal/include/lal/StringInput.h:23,
                 from FstatToplist.c:27:
In function 'strncpy',
    inlined from 'write_hs_checkpoint' at FstatToplist.c:744:3:
/opt/conda/envs/lalsuite-platform-tests/x86_64-conda-linux-gnu/sysroot/usr/include/bits/string3.h:121:10: error: '__builtin_strncpy' specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  121 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
FstatToplist.c: In function 'write_hs_checkpoint':
FstatToplist.c:738:9: note: length computed here
  738 |   len = strlen(filename)+strlen(TMP_EXT)+1;
      |         ^~~~~~~~~~~~~~~~
  CC       HeapToplist.o
cc1: all warnings being treated as errors
  CC       MCInjectHoughMulti.o
make[8]: *** [Makefile:912: FstatToplist.o] Error 1

I'd say:

  • tmpfilename is constructed explicitly to hold filename and TMP_EXT
  • so strcpy and strcat instead of strncpy and strncat should suffice

My local gcc 8.3.0 is as happy with this version as with the original, let's see what the standard pipeline here has to say and then @duncanmmacleod could try it out on that brach too.

Closes #364 (closed).

API Changes and Justification

Backwards Compatible Changes

  • This change introduces no API changes
  • This change adds new API calls

Backwards Incompatible Changes

  • This change modifies an existing API
  • This change removes an existing API

If any of the Backwards Incompatible check boxes are ticked please provide a justification why this change is necessary and why it needs to be done in a backwards incompatible way.

Review Status

@karl-wette though he's on vacation I think, but it's not urgent anyway.

Edited by Duncan Macleod

Merge request reports