From: Viktor Szakats Date: Sat, 9 Dec 2023 02:45:19 +0000 (+0000) Subject: build: fix some `-Wsign-conversion`/`-Warith-conversion` warnings X-Git-Tag: curl-8_6_0~191 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2dbe75bd7f3c36837aa06fd87a442bdf3fb7faef;p=thirdparty%2Fcurl.git build: fix some `-Wsign-conversion`/`-Warith-conversion` warnings - enable `-Wsign-conversion` warnings, but also setting them to not raise errors. - fix `-Warith-conversion` warnings seen in CI. These are triggered by `-Wsign-converion` and causing errors unless explicitly silenced. It makes more sense to fix them, there just a few of them. - fix some `-Wsign-conversion` warnings. - hide `-Wsign-conversion` warnings with a `#pragma`. - add macro `CURL_WARN_SIGN_CONVERSION` to unhide them on a per-build basis. - update a CI job to unhide them with the above macro: https://github.com/curl/curl/actions/workflows/linux.yml -> OpenSSL -O3 Closes #12492 --- diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 96f14561fb..b4ad111f80 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -111,7 +111,7 @@ jobs: - name: openssl3-O3 install_packages: zlib1g-dev valgrind install_steps: gcc-11 openssl3 - configure: CFLAGS=-O3 LDFLAGS="-Wl,-rpath,$HOME/openssl3/lib64" --with-openssl=$HOME/openssl3 --enable-debug --enable-websockets + configure: CPPFLAGS=-DCURL_WARN_SIGN_CONVERSION CFLAGS=-O3 LDFLAGS="-Wl,-rpath,$HOME/openssl3/lib64" --with-openssl=$HOME/openssl3 --enable-debug --enable-websockets singleuse: --unit - name: openssl3-clang diff --git a/CMake/PickyWarnings.cmake b/CMake/PickyWarnings.cmake index 056e56c91b..d82bbb1d6a 100644 --- a/CMake/PickyWarnings.cmake +++ b/CMake/PickyWarnings.cmake @@ -89,13 +89,12 @@ if(PICKY_COMPILER) -Wmissing-field-initializers # clang 2.7 gcc 4.1 -Wmissing-noreturn # clang 2.7 gcc 4.1 -Wno-format-nonliteral # clang 1.0 gcc 2.96 (3.0) - -Wno-sign-conversion # clang 2.9 gcc 4.3 -Wno-system-headers # clang 1.0 gcc 3.0 # -Wpadded # clang 2.9 gcc 4.1 # Not used because we cannot change public structs -Wold-style-definition # clang 2.7 gcc 3.4 -Wredundant-decls # clang 2.7 gcc 4.1 - # -Wsign-conversion # clang 2.9 gcc 4.3 # FIXME - # -Wno-error=sign-conversion # FIXME + -Wsign-conversion # clang 2.9 gcc 4.3 + -Wno-error=sign-conversion # FIXME -Wstrict-prototypes # clang 1.0 gcc 3.3 # -Wswitch-enum # clang 2.7 gcc 4.1 # Not used because this basically disallows default case -Wtype-limits # clang 2.7 gcc 4.3 diff --git a/lib/curl_setup.h b/lib/curl_setup.h index fd7358b450..551243e270 100644 --- a/lib/curl_setup.h +++ b/lib/curl_setup.h @@ -28,6 +28,13 @@ #define CURL_NO_OLDIES #endif +/* FIXME: Delete this once the warnings have been fixed. */ +#if !defined(CURL_WARN_SIGN_CONVERSION) +#ifdef __GNUC__ +#pragma GCC diagnostic ignored "-Wsign-conversion" +#endif +#endif + /* Set default _WIN32_WINNT */ #ifdef __MINGW32__ #include <_mingw.h> diff --git a/lib/doh.c b/lib/doh.c index d550df299a..ef32d507df 100644 --- a/lib/doh.c +++ b/lib/doh.c @@ -444,7 +444,7 @@ static DOHcode skipqname(const unsigned char *doh, size_t dohlen, return DOH_DNS_BAD_LABEL; if(dohlen < (*indexp + 1 + length)) return DOH_DNS_OUT_OF_RANGE; - *indexp += 1 + length; + *indexp += (unsigned int)(1 + length); } while(length); return DOH_OK; } @@ -456,14 +456,15 @@ static unsigned short get16bit(const unsigned char *doh, int index) static unsigned int get32bit(const unsigned char *doh, int index) { - /* make clang and gcc optimize this to bswap by incrementing - the pointer first. */ - doh += index; - - /* avoid undefined behavior by casting to unsigned before shifting - 24 bits, possibly into the sign bit. codegen is same, but - ub sanitizer won't be upset */ - return ( (unsigned)doh[0] << 24) | (doh[1] << 16) |(doh[2] << 8) | doh[3]; + /* make clang and gcc optimize this to bswap by incrementing + the pointer first. */ + doh += index; + + /* avoid undefined behavior by casting to unsigned before shifting + 24 bits, possibly into the sign bit. codegen is same, but + ub sanitizer won't be upset */ + return ((unsigned)doh[0] << 24) | ((unsigned)doh[1] << 16) | + ((unsigned)doh[2] << 8) | doh[3]; } static DOHcode store_a(const unsigned char *doh, int index, struct dohentry *d) diff --git a/lib/inet_pton.c b/lib/inet_pton.c index 7d3c698795..176cc956fe 100644 --- a/lib/inet_pton.c +++ b/lib/inet_pton.c @@ -112,7 +112,8 @@ inet_pton4(const char *src, unsigned char *dst) pch = strchr(digits, ch); if(pch) { - unsigned int val = *tp * 10 + (unsigned int)(pch - digits); + unsigned int val = (unsigned int)(*tp * 10) + + (unsigned int)(pch - digits); if(saw_digit && *tp == 0) return (0); diff --git a/lib/mprintf.c b/lib/mprintf.c index 514fdc87a8..3d194b24ed 100644 --- a/lib/mprintf.c +++ b/lib/mprintf.c @@ -1010,7 +1010,7 @@ number: static int addbyter(int output, FILE *data) { struct nsprintf *infop = (struct nsprintf *)data; - unsigned char outc = (unsigned char)output; + char outc = (char)output; if(infop->length < infop->max) { /* only do this if we haven't reached max length yet */ diff --git a/lib/share.c b/lib/share.c index c0a8d806f3..8fa5cda00f 100644 --- a/lib/share.c +++ b/lib/share.c @@ -133,13 +133,13 @@ curl_share_setopt(struct Curl_share *share, CURLSHoption option, ...) res = CURLSHE_BAD_OPTION; } if(!res) - share->specifier |= (1<specifier |= (unsigned int)(1<specifier &= ~(1<specifier &= ~(unsigned int)(1<specifier & (1<specifier & (unsigned int)(1<lockfunc) /* only call this if set! */ share->lockfunc(data, type, accesstype, share->clientdata); } @@ -281,7 +281,7 @@ Curl_share_unlock(struct Curl_easy *data, curl_lock_data type) if(!share) return CURLSHE_INVALID; - if(share->specifier & (1<specifier & (unsigned int)(1<unlockfunc) /* only call this if set! */ share->unlockfunc (data, type, share->clientdata); } diff --git a/lib/vauth/krb5_gssapi.c b/lib/vauth/krb5_gssapi.c index 65eb3e1b50..16b6e4037f 100644 --- a/lib/vauth/krb5_gssapi.c +++ b/lib/vauth/krb5_gssapi.c @@ -226,7 +226,8 @@ CURLcode Curl_auth_create_gssapi_security_message(struct Curl_easy *data, /* Extract the security layer and the maximum message size */ indata = output_token.value; sec_layer = indata[0]; - max_size = (indata[1] << 16) | (indata[2] << 8) | indata[3]; + max_size = ((unsigned int)indata[1] << 16) | + ((unsigned int)indata[2] << 8) | indata[3]; /* Free the challenge as it is not required anymore */ gss_release_buffer(&unused_status, &output_token); diff --git a/lib/vauth/krb5_sspi.c b/lib/vauth/krb5_sspi.c index c487149b9d..17a517a975 100644 --- a/lib/vauth/krb5_sspi.c +++ b/lib/vauth/krb5_sspi.c @@ -319,7 +319,8 @@ CURLcode Curl_auth_create_gssapi_security_message(struct Curl_easy *data, /* Extract the security layer and the maximum message size */ indata = input_buf[1].pvBuffer; sec_layer = indata[0]; - max_size = (indata[1] << 16) | (indata[2] << 8) | indata[3]; + max_size = ((unsigned long)indata[1] << 16) | + ((unsigned long)indata[2] << 8) | indata[3]; /* Free the challenge as it is not required anymore */ s_pSecFn->FreeContextBuffer(input_buf[1].pvBuffer); diff --git a/m4/curl-compilers.m4 b/m4/curl-compilers.m4 index 7968a113a9..9a4547709e 100644 --- a/m4/curl-compilers.m4 +++ b/m4/curl-compilers.m4 @@ -830,9 +830,8 @@ AC_DEFUN([CURL_SET_COMPILER_WARNING_OPTS], [ # dnl Only clang 2.9 or later if test "$compiler_num" -ge "209"; then - tmp_CFLAGS="$tmp_CFLAGS -Wno-sign-conversion" - # CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) # FIXME - # tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME + CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) + tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [shift-sign-overflow]) # CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [padded]) # Not used because we cannot change public structs fi @@ -1023,9 +1022,8 @@ AC_DEFUN([CURL_SET_COMPILER_WARNING_OPTS], [ CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [missing-parameter-type empty-body]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [clobbered ignored-qualifiers]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [conversion trampolines]) - tmp_CFLAGS="$tmp_CFLAGS -Wno-sign-conversion" - # CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) # FIXME - # tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME + CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) + tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [vla]) dnl required for -Warray-bounds, included in -Wall tmp_CFLAGS="$tmp_CFLAGS -ftree-vrp" diff --git a/tests/libtest/first.c b/tests/libtest/first.c index 6d13806887..42c53c6941 100644 --- a/tests/libtest/first.c +++ b/tests/libtest/first.c @@ -65,12 +65,16 @@ int select_wrapper(int nfds, fd_set *rd, fd_set *wr, fd_set *exc, void wait_ms(int ms) { + if(ms < 0) + return; #ifdef USE_WINSOCK - Sleep(ms); + Sleep((DWORD)ms); #else - struct timeval t; - curlx_mstotv(&t, ms); - select_wrapper(0, NULL, NULL, NULL, &t); + { + struct timeval t; + curlx_mstotv(&t, ms); + select_wrapper(0, NULL, NULL, NULL, &t); + } #endif } diff --git a/tests/libtest/lib1560.c b/tests/libtest/lib1560.c index 97d02daeff..6e734d2bf2 100644 --- a/tests/libtest/lib1560.c +++ b/tests/libtest/lib1560.c @@ -1045,7 +1045,7 @@ static CURLUcode updateurl(CURLU *u, const char *cmd, unsigned int setflags) while(p) { char *e = strchr(p, ','); if(e) { - size_t n = e-p; + size_t n = (size_t)(e - p); char buf[80]; char part[80]; char value[80]; diff --git a/tests/libtest/lib1947.c b/tests/libtest/lib1947.c index b7a013127a..c81345f9db 100644 --- a/tests/libtest/lib1947.c +++ b/tests/libtest/lib1947.c @@ -39,7 +39,7 @@ int test(char *URL) CURLcode res = CURLE_OK; struct curl_header *h; int count = 0; - int origins; + unsigned int origins; global_init(CURL_GLOBAL_DEFAULT); diff --git a/tests/libtest/testutil.c b/tests/libtest/testutil.c index 4a3cd944af..efbbf9019c 100644 --- a/tests/libtest/testutil.c +++ b/tests/libtest/testutil.c @@ -37,8 +37,8 @@ struct timeval tutil_tvnow(void) */ struct timeval now; DWORD milliseconds = GetTickCount(); - now.tv_sec = milliseconds / 1000; - now.tv_usec = (milliseconds % 1000) * 1000; + now.tv_sec = (long)(milliseconds / 1000); + now.tv_usec = (long)((milliseconds % 1000) * 1000); return now; } diff --git a/tests/server/mqttd.c b/tests/server/mqttd.c index 0c31376c10..a85f9f80d9 100644 --- a/tests/server/mqttd.c +++ b/tests/server/mqttd.c @@ -569,7 +569,7 @@ static curl_socket_t mqttit(curl_socket_t fd) goto end; } /* ignore the connect flag byte and two keepalive bytes */ - payload_len = (buffer[10] << 8) | buffer[11]; + payload_len = (size_t)(buffer[10] << 8) | buffer[11]; /* first part of the payload is the client ID */ client_id_length = payload_len; @@ -579,14 +579,16 @@ static curl_socket_t mqttit(curl_socket_t fd) start_usr = client_id_offset + payload_len; if(usr_flag == (unsigned char)(conn_flags & usr_flag)) { logmsg("User flag is present in CONN flag"); - payload_len += (buffer[start_usr] << 8) | buffer[start_usr + 1]; + payload_len += (size_t)(buffer[start_usr] << 8) | + buffer[start_usr + 1]; payload_len += 2; /* MSB and LSB for user length */ } start_passwd = client_id_offset + payload_len; if(passwd_flag == (char)(conn_flags & passwd_flag)) { logmsg("Password flag is present in CONN flags"); - payload_len += (buffer[start_passwd] << 8) | buffer[start_passwd + 1]; + payload_len += (size_t)(buffer[start_passwd] << 8) | + buffer[start_passwd + 1]; payload_len += 2; /* MSB and LSB for password length */ } @@ -631,7 +633,7 @@ static curl_socket_t mqttit(curl_socket_t fd) packet_id = (unsigned short)((buffer[0] << 8) | buffer[1]); /* two bytes topic length */ - topic_len = (buffer[2] << 8) | buffer[3]; + topic_len = (size_t)(buffer[2] << 8) | buffer[3]; if(topic_len != (remaining_length - 5)) { logmsg("Wrong topic length, got %u expected %zu", topic_len, remaining_length - 5); @@ -676,7 +678,7 @@ static curl_socket_t mqttit(curl_socket_t fd) logprotocol(FROM_CLIENT, "PUBLISH", remaining_length, dump, buffer, rc); - topiclen = (buffer[1 + bytes] << 8) | buffer[2 + bytes]; + topiclen = (size_t)(buffer[1 + bytes] << 8) | buffer[2 + bytes]; logmsg("Got %zu bytes topic", topiclen); /* TODO: verify topiclen */ diff --git a/tests/server/util.c b/tests/server/util.c index d6698e5e36..74d6d08072 100644 --- a/tests/server/util.c +++ b/tests/server/util.c @@ -137,7 +137,7 @@ void logmsg(const char *msg, ...) static const char *win32_strerror(int err, char *buf, size_t buflen) { if(!FormatMessageA((FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS), NULL, err, + FORMAT_MESSAGE_IGNORE_INSERTS), NULL, (DWORD)err, LANG_NEUTRAL, buf, (DWORD)buflen, NULL)) msnprintf(buf, buflen, "Unknown error %d (%#x)", err, err); return buf; @@ -247,7 +247,7 @@ int wait_ms(int timeout_ms) #if defined(MSDOS) delay(timeout_ms); #elif defined(USE_WINSOCK) - Sleep(timeout_ms); + Sleep((DWORD)timeout_ms); #else pending_ms = timeout_ms; initial_tv = tvnow(); diff --git a/tests/unit/unit1651.c b/tests/unit/unit1651.c index 58d2f10762..63f3393947 100644 --- a/tests/unit/unit1651.c +++ b/tests/unit/unit1651.c @@ -368,7 +368,7 @@ UNITTEST_START happens */ for(byte = 1 ; byte < 255; byte += 17) { for(i = 0; i < 45; i++) { - char backup = cert[i]; + unsigned char backup = cert[i]; cert[i] = (unsigned char) (byte & 0xff); (void) Curl_extract_certinfo(data, 0, beg, end); cert[i] = backup;