New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
autotools: stop setting -std=gnu89
with --enable-warnings
#12346
Conversation
75a5954
to
56318f9
Compare
- 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`. ``` In file included from ../../tests/libtest/test.h:43, from curlcheck.h:24, from unit1395.c:24: unit1395.c: In function ‘test’: ../../lib/curl_printf.h:44:18: error: ‘%s’ directive argument is null [-Werror=format-overflow=] 44 | # define fprintf curl_mfprintf unit1395.c:93:7: note: in expansion of macro ‘fprintf’ 93 | fprintf(stderr, "Test %u: '%s' gave '%s' instead of NULL\n", | ^~~~~~~ ``` Ref: https://github.com/curl/curl/actions/runs/6896854253/job/18763836775?pr=12346#step:33:1689
``` vssh/libssh.c: In function ‘myssh_statemach_act’: vssh/libssh.c:1162:29: error: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Werror=format=] 1162 | char *tmp = aprintf("statvfs:\n" | ^~~~~~~~~~~~ ...... 1169 | statvfs->f_bsize, statvfs->f_frsize, | ~~~~~~~~~~~~~~~~ | | | uint64_t {aka long unsigned int} vssh/libssh.c:1163:42: note: format string is defined here 1163 | "f_bsize: %llu\n" "f_frsize: %llu\n" | ~~~^ | | | long long unsigned int | %lu ``` Ref: https://github.com/curl/curl/actions/runs/6896854253/job/18763838007?pr=12346#step:29:895
``` In file included from curl_setup.h:656, from conncache.c:26: conncache.c: In function ‘Curl_conncache_add_conn’: conncache.c:246:22: error: format ‘%ld’ expects argument of type ‘long int’, but argument 3 has type ‘curl_off_t’ {aka ‘long long int’} [-Werror=format=] 246 | DEBUGF(infof(data, "Added connection %ld. " | ^~~~~~~~~~~~~~~~~~~~~~~~ 247 | "The cache now contains %zu members", 248 | conn->connection_id, connc->num_conn)); | ~~~~~~~~~~~~~~~~~~~ | | | curl_off_t {aka long long int} curl_setup_once.h:286:19: note: in definition of macro ‘DEBUGF’ 286 | #define DEBUGF(x) x | ^ conncache.c:246:10: note: in expansion of macro ‘infof’ 246 | DEBUGF(infof(data, "Added connection %ld. " | ^~~~~ conncache.c:246:42: note: format string is defined here 246 | DEBUGF(infof(data, "Added connection %ld. " | ~~^ | | | long int | %lld ``` Ref: https://github.com/curl/curl/actions/runs/6896854263/job/18763831142?pr=12346#step:6:67
``` http2.c: In function 'nw_out_writer': http2.c:374:14: error: null pointer dereference [-Werror=null-dereference] 374 | nwritten = Curl_conn_cf_send(cf->next, data, (const char *)buf, buflen, err); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` Ref: https://github.com/curl/curl/actions/runs/6896854253/job/18763839238?pr=12346#step:30:214
E.g.: ``` ftpgetinfo.c: In function 'main': ftpgetinfo.c:78:16: error: ISO C does not support the 'I' printf flag [-Werror=format=] 78 | printf("filesize %s: %" CURL_FORMAT_CURL_OFF_T " bytes\n", | ^~~~~~~~~~~~~~~~ ftpgetinfo.c:78:16: error: format '%d' expects argument of type 'int', but argument 3 has type 'curl_off_t' {aka 'long long int'} [-Werror=format=] 78 | printf("filesize %s: %" CURL_FORMAT_CURL_OFF_T " bytes\n", | ^~~~~~~~~~~~~~~~ 79 | filename, filesize); | ~~~~~~~~ | | | curl_off_t {aka long long int} In file included from ../../include/curl/curl.h:54, from ftpgetinfo.c:27: ../../include/curl/system.h:205:42: note: format string is defined here 205 | # define CURL_FORMAT_CURL_OFF_T "I64d" ``` Ref: https://dev.azure.com/daniel0244/curl/_build/results?buildId=18581&view=logs&jobId=ccf9cc6d-2ef1-5cf2-2c09-30f0c14f923b
Caveat: mingw old, I have no info if `inttypes.h` is supported there.
No longer necessary after fixing curl public headers.
56318f9
to
915fb74
Compare
The original goal of this option was to test C89-compliance. Keep that feature, but limited to a specific CI job. We can extended to other gcc jobs as needed. Don't add them to these though, as in these revealed actual issues by building without `-std=gnu89`: - openssl3-O3 - event-based - Slackware-openssl-with-gssapi-gcc - mingw-w64 builds
Restored |
This old old-mingw header package (from 2010) does contain It means old-mingw likely continues to work fine with public headers after this change, though I haven't actually tried. |
71385a3
to
547fb72
Compare
I couldn't replicate the same issue with libssh2 [1]. I cannot even grasp it for libssh, as it seems rooted from the fact that Anyway, for libssh2 the MSVC-issue seems to apply too: Lines 1965 to 1971 in a9fd0d0
Any thoughts to include this MSVC fix for libssh2 too?: diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c
index 543b6fb43..4eb5cf63f 100644
--- a/lib/vssh/libssh2.c
+++ b/lib/vssh/libssh2.c
@@ -1962,13 +1962,23 @@ static CURLcode ssh_statemach_act(struct Curl_easy *data, bool *block)
break;
}
else if(rc == 0) {
+ #ifdef _MSC_VER
+ #define LIBSSH2_VFS_SIZE_MASK "I64u"
+ #else
+ #define LIBSSH2_VFS_SIZE_MASK "llu"
+ #endif
char *tmp = aprintf("statvfs:\n"
- "f_bsize: %llu\n" "f_frsize: %llu\n"
- "f_blocks: %llu\n" "f_bfree: %llu\n"
- "f_bavail: %llu\n" "f_files: %llu\n"
- "f_ffree: %llu\n" "f_favail: %llu\n"
- "f_fsid: %llu\n" "f_flag: %llu\n"
- "f_namemax: %llu\n",
+ "f_bsize: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_frsize: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_blocks: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_bfree: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_bavail: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_files: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_ffree: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_favail: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_fsid: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_flag: %" LIBSSH2_VFS_SIZE_MASK "\n"
+ "f_namemax: %" LIBSSH2_VFS_SIZE_MASK "\n",
statvfs.f_bsize, statvfs.f_frsize,
statvfs.f_blocks, statvfs.f_bfree,
statvfs.f_bavail, statvfs.f_files, |
Applying the same fix as used with libssh.
547fb72
to
a972c26
Compare
To make libssh/libssh2 masks the same and future-proof, I'm thinking of moving from: #ifdef _MSC_VER
#define LIBSSH_VFS_SIZE_MASK "I64u"
#else
#define LIBSSH_VFS_SIZE_MASK PRIu64
#endif libssh2: #ifdef _MSC_VER
#define LIBSSH2_VFS_SIZE_MASK "I64u"
#else
#define LIBSSH2_VFS_SIZE_MASK "llu"
#endif to this for both: #ifdef PRIu64
#define LIBSSH_VFS_SIZE_MASK PRIu64
#elif defined(_MSC_VER)
#define LIBSSH_VFS_SIZE_MASK "I64u"
#else
#define LIBSSH_VFS_SIZE_MASK "llu"
#endif This would use the official ...well, maybe just for libssh. For libssh2 the 64-bit type UPDATE: Talked myself out of this. Once libssh gets a release with universal |
Avoid stomping the libssh and libssh2 macro namespaces by prefixing these with `CURL_`. Follow-up to 413a0fe curl#12346 Closes #xxxxx
Do not alter the C standard when building with
--enable-warnings
whenbuilding 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
andCURL_FORMAT_CURL_OFF_TU
with mingw-w64and 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]:strcmp()
with NULL to avoid undefined behaviour.%s
.ci: keep a build job with
-std=gnu89
to continue testing forC89-compliance. We can apply this to other gcc jobs as needed.
Ref: b23ce2c (2022-09-23) curl-compilers.m4: for gcc + want warnings, set gnu89 standard #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