[2024.4.0] warnings during the build

[picca]

Hello, I report here a bunch of warning observed during the Debian build

[6/20] cc -Isrc/fabio/ext/cf_io.cpython-312-s390x-linux-gnu.so.p -Isrc/fabio/ext -I../src/fabio/ext -I../src/fabio/ext/include -I/usr/include/python3.12 -fvisibility=hidden -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O3 -g -O2 -Werror=implicit-function-declaration -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -MD -MQ src/fabio/ext/cf_io.cpython-312-s390x-linux-gnu.so.p/src_columnfile.c.o -MF src/fabio/ext/cf_io.cpython-312-s390x-linux-gnu.so.p/src_columnfile.c.o.d -o src/fabio/ext/cf_io.cpython-312-s390x-linux-gnu.so.p/src_columnfile.c.o -c ../src/fabio/ext/src/columnfile.c
../src/fabio/ext/src/columnfile.c: In function ‘cf_read_ascii’:
../src/fabio/ext/src/columnfile.c:200:3: warning: ignoring return value of ‘fgets’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  200 |   fgets(line,2048,(FILE *)fp);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/fabio/ext/src/columnfile.c:19:20: warning: array subscript 32 is outside array bounds of ‘char *[32]’ [-Warray-bounds=]
   19 | #define sscanf_s   sscanf
      |                    ^
../src/fabio/ext/src/columnfile.c:161:9: note: in expansion of macro ‘sscanf_s’
  161 |   ncols=sscanf_s(line,hdr_ctl,repeat16_inc(clabels,0),repeat16_inc(clabels,16),*(clabels+32));
      |         ^~~~~~~~
../src/fabio/ext/src/columnfile.c:155:20: note: at offset 256 into object of size 256 allocated by ‘malloc’
  155 |   clabels=(char**) malloc(CF_INIT_COLS* sizeof(char*));
      |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Cheers

[kif]

@jonwright I managed to fix the warning about fgets, but I have no clue about the sscanf ... especially what is the rational of *(clabels+32) where clabels is an array of char of 32 elements. Since you wrote this piece of code, could you please give me some hint on the reason why you did that ?

[jonwright]

In practice, I didn't know anyone using this code, so maybe the sensible thing is just to remove it.

In ImageD11 we have https://github.com/jonwright/ImageD11/blob/master/ImageD11/columnfile.py and we are trying to move these data into hdf5 anyway.

What I interpret the code is doing:

ncols=sscanf_s(line,hdr_ctl,repeat16_inc(clabels,0),repeat16_inc(clabels,16),*(clabels+32));

/* ncols will be set to the number of items read */
/* hdr_ctl is a format string # %s %s ... repeated 34 times (so a few too many) */
/* repeat16_inc(clabels[0]) expands to tell sscanf to read the strings into the clabels array by giving blocks of 16 args */

To fix I think the string hdr_ctl needs max 32 entries and then the last argument *(clabels+32) could be deleted. But if we don't have test coverage it makes more sense to get rid of all of it...

[jonwright]

There is a draft pull request / diff over here https://github.com/jonwright/fabio/tree/jonwright-patch-1 , but the option to just remove it all seems to make more sense. What do you think?

[kif]

Thanks, I took your modifications into #572. If someone is using this code and it breaks, he will complain an provide material for a non regression test.