madler / zlib

A massively spiffy yet delicately unobtrusive compression library.
http://zlib.net/
Other
5.55k stars 2.42k forks source link

Compiler warning with fileno when building with non-gnu C extensions #926

Closed pmqs closed 6 months ago

pmqs commented 6 months ago

Building zlib throws up this compiler warning when the non-gnu extensions are enabled - so that is with gcc & clang using the -std option with any of the values c99, c11, c17, and c2x. This issue was found using the workflow file contained in #925

gcc -std=c2x -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -I. -c -o minigzip.o test/minigzip.c
test/minigzip.c: In function ‘main’:
test/minigzip.c:538:28: warning: implicit declaration of function ‘fileno’ [-Wimplicit-function-declaration]
  538 |             file = gzdopen(fileno(stdin), "rb");
      |                            ^~~~~~
gcc -std=c2x -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN  -o minigzip minigzip.o -L. libz.a
gcc -std=c2x -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -o minigzipsh minigzip.o  -L. libz.so.1.3.1.1-motley
gcc -std=c2x -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -I. -D_FILE_OFFSET_BITS=64 -c -o minigzip64.o test/minigzip.c
test/minigzip.c: In function ‘main’:
test/minigzip.c:538:28: warning: implicit declaration of function ‘fileno’ [-Wimplicit-function-declaration]
  538 |             file = gzdopen(fileno(stdin), "rb");
      |                            ^~~~~~
gcc -std=c2x -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN  -o minigzip64 minigzip64.o -L. libz.a

Looking at fileno in stdio.h on my Ubuntu setup I see it is guarded by __USE_POSIX

#ifdef  __USE_POSIX
/* Return the system file descriptor for STREAM.  */
extern int fileno (FILE *__stream) __THROW __wur;
#endif /* Use POSIX.  */

This change mirrors what is done in gzguts.h to define _POSIX_SOURCE if it isn't already defined.

madler commented 6 months ago

Applied.

pmqs commented 6 months ago

Applied.

@madler, just spotted a failure in the MacOS clang build with this change (see https://github.com/madler/zlib/actions/runs/7789724438/job/21241964719 and https://github.com/madler/zlib/actions/runs/7789724436/job/21241964381)

clang -fPIC -DHAVE_HIDDEN  -DPIC -c -o objs/adler32.o adler32.c
test/minigzip.c:410:5: error: implicitly declaring library function 'snprintf' with type 'int (char *, unsigned long, const char *, ...)' [-Werror,-Wimplicit-function-declaration]
    snprintf(outfile, sizeof(outfile), "%s%s", file, GZ_SUFFIX);
    ^
test/minigzip.c:410:5: note: include the header <stdio.h> or explicitly provide a declaration for 'snprintf'
1 error generated.

Backing out this change makes the issue go away.

Interestingly the Mac builds using gcc work fine with this change (see https://github.com/madler/zlib/actions/runs/7789724436/job/21241964567).

Does clang on a Mac differ from the Linux version enough to mean that this isn't an issue on Linux, but is on Mac?

madler commented 6 months ago

Yes, I develop on a Mac. I changed it to # define _POSIX_C_SOURCE 200809L.

madler commented 6 months ago

After some testing, I changed that to 200112L so it would work on Solaris.

pmqs commented 6 months ago

@madler I merged the 200112L into my fork & ran the workflow in #925 with this change included

All is fine apart from the combination of Ubuntu + clang + cmake with c89 or iso9899:199409

Error in both cases is

 [ 18%] Building C object CMakeFiles/zlib.dir/gzlib.c.o
/home/runner/work/zlib/zlib/gzlib.c:204:15: error: implicitly declaring library function 'snprintf' with type 'int (char *, unsigned long, const char *, ...)' [-Werror,-Wimplicit-function-declaration]
        (void)snprintf(state->path, len + 1, "%s", (const char *)path);
              ^
/home/runner/work/zlib/zlib/gzlib.c:204:15: note: include the header <stdio.h> or explicitly provide a declaration for 'snprintf'
1 error generated.

It does work with the configure build, so the issue must to be something in the generation of zconf.h and/or the command-line options passed to the C compiler.

Right - I see the issue. The configure build adds -DNO_vsnprintf but the cmake build does not. Adding that by hand makes the issue go away.

Don't see any code in CMakeLists.txt that runs test compilations that I can use as inspiration. I'll add an issue for this edge case.