curl-compilers.m4: for gcc + want warnings, set gnu89 standard #9542
Conversation
|
This will be a problem with third-party libraries using features from later standards, like C++ comments, in their public headers. Maybe add it to some CI builds instead? |
|
yeah, but more importantly this identifies numerous places within libcurl itself that are not proper C89. That was my primary thought with this, to try it out. It might not make sense to land in the short term. The question also comes down to: are those nits worth fixing if nobody has suffered from any problems with them while we are slowly considering bumping to C99 in a future... |
bagder
force-pushed
the
bagder/gcc-gnu89
branch
2 times, most recently
from
f51f81a
to
a4ae62d
Compare
|
I decided they are probably worth fixing because: If (and this has not been decided yet) we change the requirement to C99 at some point, then whatever is in git before that switch will be the last C89 compliant curl code and it could be worth having that in a fine shape. Compliance wise. |
|
FYI: This exhibited problems fixed (sucessfully) in 6fbf7d3, 9eec754 and c3e634d: no compiler message for the current master. I had to define As a consequence we might question ourselves if curl's C89 requirement only targets the language or also the libraries. |
|
C89 doesn't provide nearly enough of an API to build curl (e.g. sockets) so
it's safe to assume that we're talking about language features only. Various
APIs outside C89 are also required (e.g. POSIX). I used to run an autobuild that
tried to build with a minimum set of such APIs (by setting things like
-D_XOPEN_SOURCE=600) to try to make sure that it wouldn't depend on too much
more. It might be interesting to turn that into a CI build.
|
|
|
Agreed: see the comment about |
Yes exactly: language features only. Everything else we can detect and adapt to. |
bagder
force-pushed
the
bagder/gcc-gnu89
branch
from
6ea3942
to
fa55f9d
Compare
To better verify that the code is C89
bagder
force-pushed
the
bagder/gcc-gnu89
branch
from
fa55f9d
to
cbbc315
Compare
Do not alter the C standard when building with `--enable-warnings` when building with gcc. On one hand this alters warning results compared to a default build. On the other, it may produce different binaries, which is unexpected. Also fix new warnings that appeared after removing `-std=gnu89`: - include: fix public curl headers to use the correct printf mask for `CURL_FORMAT_CURL_OFF_T` and `CURL_FORMAT_CURL_OFF_TU` with mingw-w64 and Visual Studio 2013 and newer. This fixes the printf mask warnings in examples and tests. E.g. [1] - conncache: fix printf format string [2]. - http2: fix potential null pointer dereference [3]. (seen on Slackware with gcc 11.) - libssh: fix printf format string in SFTP code [4]. Also make MSVC builds compatible with old CRT versions. - libssh2: fix printf format string in SFTP code for MSVC. Applying the same fix as for libssh above. - unit1395: fix `argument is null` and related issues [5]: - stop calling `strcmp()` with NULL to avoid undefined behaviour. - fix checking results if some of them were NULL. - do not pass NULL to printf `%s`. - ci: keep a build job with `-std=gnu89` to continue testing for C89-compliance. We can apply this to other gcc jobs as needed. Ref: b23ce2c (2022-09-23) #9542 [1] https://dev.azure.com/daniel0244/curl/_build/results?buildId=18581&view=logs&jobId=ccf9cc6d-2ef1-5cf2-2c09-30f0c14f923b [2] https://github.com/curl/curl/actions/runs/6896854263/job/18763831142?pr=12346#step:6:67 [3] https://github.com/curl/curl/actions/runs/6896854253/job/18763839238?pr=12346#step:30:214 [4] https://github.com/curl/curl/actions/runs/6896854253/job/18763838007?pr=12346#step:29:895 [5] https://github.com/curl/curl/actions/runs/6896854253/job/18763836775?pr=12346#step:33:1689 Closes #12346
To better verify that the code is C89