Skip to content

GitLab

Merged
Created by David Keitel@david-keitelDeveloper1 of 4 tasks completed1/4 tasks

FstatToplist: simplify string ops in write_hs_checkpoint()

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