autotools: stop setting -std=gnu89 with --enable-warnings#12346
autotools: stop setting -std=gnu89 with --enable-warnings#12346vszakats wants to merge 11 commits intocurl:masterfrom
-std=gnu89 with --enable-warnings#12346Conversation
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
#endiflibssh2: #ifdef _MSC_VER
#define LIBSSH2_VFS_SIZE_MASK "I64u"
#else
#define LIBSSH2_VFS_SIZE_MASK "llu"
#endifto 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"
#endifThis 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-warningswhenbuilding 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_TandCURL_FORMAT_CURL_OFF_TUwith 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 nulland related issues [5]:strcmp()with NULL to avoid undefined behaviour.%s.ci: keep a build job with
-std=gnu89to 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