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
This commit is contained in:
Viktor Szakats 2023-12-09 02:45:19 +00:00
parent 68d80a8f9b
commit 2dbe75bd7f
No known key found for this signature in database
GPG Key ID: B5ABD165E2AEF201
17 changed files with 56 additions and 42 deletions

View File

@ -111,7 +111,7 @@ jobs:
- name: openssl3-O3 - name: openssl3-O3
install_packages: zlib1g-dev valgrind install_packages: zlib1g-dev valgrind
install_steps: gcc-11 openssl3 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 singleuse: --unit
- name: openssl3-clang - name: openssl3-clang

View File

@ -89,13 +89,12 @@ if(PICKY_COMPILER)
-Wmissing-field-initializers # clang 2.7 gcc 4.1 -Wmissing-field-initializers # clang 2.7 gcc 4.1
-Wmissing-noreturn # 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-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 -Wno-system-headers # clang 1.0 gcc 3.0
# -Wpadded # clang 2.9 gcc 4.1 # Not used because we cannot change public structs # -Wpadded # clang 2.9 gcc 4.1 # Not used because we cannot change public structs
-Wold-style-definition # clang 2.7 gcc 3.4 -Wold-style-definition # clang 2.7 gcc 3.4
-Wredundant-decls # clang 2.7 gcc 4.1 -Wredundant-decls # clang 2.7 gcc 4.1
# -Wsign-conversion # clang 2.9 gcc 4.3 # FIXME -Wsign-conversion # clang 2.9 gcc 4.3
# -Wno-error=sign-conversion # FIXME -Wno-error=sign-conversion # FIXME
-Wstrict-prototypes # clang 1.0 gcc 3.3 -Wstrict-prototypes # clang 1.0 gcc 3.3
# -Wswitch-enum # clang 2.7 gcc 4.1 # Not used because this basically disallows default case # -Wswitch-enum # clang 2.7 gcc 4.1 # Not used because this basically disallows default case
-Wtype-limits # clang 2.7 gcc 4.3 -Wtype-limits # clang 2.7 gcc 4.3

View File

@ -28,6 +28,13 @@
#define CURL_NO_OLDIES #define CURL_NO_OLDIES
#endif #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 */ /* Set default _WIN32_WINNT */
#ifdef __MINGW32__ #ifdef __MINGW32__
#include <_mingw.h> #include <_mingw.h>

View File

@ -444,7 +444,7 @@ static DOHcode skipqname(const unsigned char *doh, size_t dohlen,
return DOH_DNS_BAD_LABEL; return DOH_DNS_BAD_LABEL;
if(dohlen < (*indexp + 1 + length)) if(dohlen < (*indexp + 1 + length))
return DOH_DNS_OUT_OF_RANGE; return DOH_DNS_OUT_OF_RANGE;
*indexp += 1 + length; *indexp += (unsigned int)(1 + length);
} while(length); } while(length);
return DOH_OK; 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) static unsigned int get32bit(const unsigned char *doh, int index)
{ {
/* make clang and gcc optimize this to bswap by incrementing /* make clang and gcc optimize this to bswap by incrementing
the pointer first. */ the pointer first. */
doh += index; doh += index;
/* avoid undefined behavior by casting to unsigned before shifting /* avoid undefined behavior by casting to unsigned before shifting
24 bits, possibly into the sign bit. codegen is same, but 24 bits, possibly into the sign bit. codegen is same, but
ub sanitizer won't be upset */ ub sanitizer won't be upset */
return ( (unsigned)doh[0] << 24) | (doh[1] << 16) |(doh[2] << 8) | doh[3]; 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) static DOHcode store_a(const unsigned char *doh, int index, struct dohentry *d)

View File

@ -112,7 +112,8 @@ inet_pton4(const char *src, unsigned char *dst)
pch = strchr(digits, ch); pch = strchr(digits, ch);
if(pch) { 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) if(saw_digit && *tp == 0)
return (0); return (0);

View File

@ -1010,7 +1010,7 @@ number:
static int addbyter(int output, FILE *data) static int addbyter(int output, FILE *data)
{ {
struct nsprintf *infop = (struct nsprintf *)data; struct nsprintf *infop = (struct nsprintf *)data;
unsigned char outc = (unsigned char)output; char outc = (char)output;
if(infop->length < infop->max) { if(infop->length < infop->max) {
/* only do this if we haven't reached max length yet */ /* only do this if we haven't reached max length yet */

View File

@ -133,13 +133,13 @@ curl_share_setopt(struct Curl_share *share, CURLSHoption option, ...)
res = CURLSHE_BAD_OPTION; res = CURLSHE_BAD_OPTION;
} }
if(!res) if(!res)
share->specifier |= (1<<type); share->specifier |= (unsigned int)(1<<type);
break; break;
case CURLSHOPT_UNSHARE: case CURLSHOPT_UNSHARE:
/* this is a type this share will no longer share */ /* this is a type this share will no longer share */
type = va_arg(param, int); type = va_arg(param, int);
share->specifier &= ~(1<<type); share->specifier &= ~(unsigned int)(1<<type);
switch(type) { switch(type) {
case CURL_LOCK_DATA_DNS: case CURL_LOCK_DATA_DNS:
break; break;
@ -264,7 +264,7 @@ Curl_share_lock(struct Curl_easy *data, curl_lock_data type,
if(!share) if(!share)
return CURLSHE_INVALID; return CURLSHE_INVALID;
if(share->specifier & (1<<type)) { if(share->specifier & (unsigned int)(1<<type)) {
if(share->lockfunc) /* only call this if set! */ if(share->lockfunc) /* only call this if set! */
share->lockfunc(data, type, accesstype, share->clientdata); share->lockfunc(data, type, accesstype, share->clientdata);
} }
@ -281,7 +281,7 @@ Curl_share_unlock(struct Curl_easy *data, curl_lock_data type)
if(!share) if(!share)
return CURLSHE_INVALID; return CURLSHE_INVALID;
if(share->specifier & (1<<type)) { if(share->specifier & (unsigned int)(1<<type)) {
if(share->unlockfunc) /* only call this if set! */ if(share->unlockfunc) /* only call this if set! */
share->unlockfunc (data, type, share->clientdata); share->unlockfunc (data, type, share->clientdata);
} }

View File

@ -226,7 +226,8 @@ CURLcode Curl_auth_create_gssapi_security_message(struct Curl_easy *data,
/* Extract the security layer and the maximum message size */ /* Extract the security layer and the maximum message size */
indata = output_token.value; indata = output_token.value;
sec_layer = indata[0]; 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 */ /* Free the challenge as it is not required anymore */
gss_release_buffer(&unused_status, &output_token); gss_release_buffer(&unused_status, &output_token);

View File

@ -319,7 +319,8 @@ CURLcode Curl_auth_create_gssapi_security_message(struct Curl_easy *data,
/* Extract the security layer and the maximum message size */ /* Extract the security layer and the maximum message size */
indata = input_buf[1].pvBuffer; indata = input_buf[1].pvBuffer;
sec_layer = indata[0]; 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 */ /* Free the challenge as it is not required anymore */
s_pSecFn->FreeContextBuffer(input_buf[1].pvBuffer); s_pSecFn->FreeContextBuffer(input_buf[1].pvBuffer);

View File

@ -830,9 +830,8 @@ AC_DEFUN([CURL_SET_COMPILER_WARNING_OPTS], [
# #
dnl Only clang 2.9 or later dnl Only clang 2.9 or later
if test "$compiler_num" -ge "209"; then if test "$compiler_num" -ge "209"; then
tmp_CFLAGS="$tmp_CFLAGS -Wno-sign-conversion" CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion])
# CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) # FIXME tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME
# tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME
CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [shift-sign-overflow]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [shift-sign-overflow])
# CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [padded]) # Not used because we cannot change public structs # CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [padded]) # Not used because we cannot change public structs
fi 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], [missing-parameter-type empty-body])
CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [clobbered ignored-qualifiers]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [clobbered ignored-qualifiers])
CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [conversion trampolines]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [conversion trampolines])
tmp_CFLAGS="$tmp_CFLAGS -Wno-sign-conversion" CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion])
# CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [sign-conversion]) # FIXME tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME
# tmp_CFLAGS="$tmp_CFLAGS -Wno-error=sign-conversion" # FIXME
CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [vla]) CURL_ADD_COMPILER_WARNINGS([tmp_CFLAGS], [vla])
dnl required for -Warray-bounds, included in -Wall dnl required for -Warray-bounds, included in -Wall
tmp_CFLAGS="$tmp_CFLAGS -ftree-vrp" tmp_CFLAGS="$tmp_CFLAGS -ftree-vrp"

View File

@ -65,12 +65,16 @@ int select_wrapper(int nfds, fd_set *rd, fd_set *wr, fd_set *exc,
void wait_ms(int ms) void wait_ms(int ms)
{ {
if(ms < 0)
return;
#ifdef USE_WINSOCK #ifdef USE_WINSOCK
Sleep(ms); Sleep((DWORD)ms);
#else #else
struct timeval t; {
curlx_mstotv(&t, ms); struct timeval t;
select_wrapper(0, NULL, NULL, NULL, &t); curlx_mstotv(&t, ms);
select_wrapper(0, NULL, NULL, NULL, &t);
}
#endif #endif
} }

View File

@ -1045,7 +1045,7 @@ static CURLUcode updateurl(CURLU *u, const char *cmd, unsigned int setflags)
while(p) { while(p) {
char *e = strchr(p, ','); char *e = strchr(p, ',');
if(e) { if(e) {
size_t n = e-p; size_t n = (size_t)(e - p);
char buf[80]; char buf[80];
char part[80]; char part[80];
char value[80]; char value[80];

View File

@ -39,7 +39,7 @@ int test(char *URL)
CURLcode res = CURLE_OK; CURLcode res = CURLE_OK;
struct curl_header *h; struct curl_header *h;
int count = 0; int count = 0;
int origins; unsigned int origins;
global_init(CURL_GLOBAL_DEFAULT); global_init(CURL_GLOBAL_DEFAULT);

View File

@ -37,8 +37,8 @@ struct timeval tutil_tvnow(void)
*/ */
struct timeval now; struct timeval now;
DWORD milliseconds = GetTickCount(); DWORD milliseconds = GetTickCount();
now.tv_sec = milliseconds / 1000; now.tv_sec = (long)(milliseconds / 1000);
now.tv_usec = (milliseconds % 1000) * 1000; now.tv_usec = (long)((milliseconds % 1000) * 1000);
return now; return now;
} }

View File

@ -569,7 +569,7 @@ static curl_socket_t mqttit(curl_socket_t fd)
goto end; goto end;
} }
/* ignore the connect flag byte and two keepalive bytes */ /* 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 */ /* first part of the payload is the client ID */
client_id_length = payload_len; 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; start_usr = client_id_offset + payload_len;
if(usr_flag == (unsigned char)(conn_flags & usr_flag)) { if(usr_flag == (unsigned char)(conn_flags & usr_flag)) {
logmsg("User flag is present in CONN 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 */ payload_len += 2; /* MSB and LSB for user length */
} }
start_passwd = client_id_offset + payload_len; start_passwd = client_id_offset + payload_len;
if(passwd_flag == (char)(conn_flags & passwd_flag)) { if(passwd_flag == (char)(conn_flags & passwd_flag)) {
logmsg("Password flag is present in CONN flags"); 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 */ 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]); packet_id = (unsigned short)((buffer[0] << 8) | buffer[1]);
/* two bytes topic length */ /* 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)) { if(topic_len != (remaining_length - 5)) {
logmsg("Wrong topic length, got %u expected %zu", logmsg("Wrong topic length, got %u expected %zu",
topic_len, remaining_length - 5); topic_len, remaining_length - 5);
@ -676,7 +678,7 @@ static curl_socket_t mqttit(curl_socket_t fd)
logprotocol(FROM_CLIENT, "PUBLISH", remaining_length, logprotocol(FROM_CLIENT, "PUBLISH", remaining_length,
dump, buffer, rc); 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); logmsg("Got %zu bytes topic", topiclen);
/* TODO: verify topiclen */ /* TODO: verify topiclen */

View File

@ -137,7 +137,7 @@ void logmsg(const char *msg, ...)
static const char *win32_strerror(int err, char *buf, size_t buflen) static const char *win32_strerror(int err, char *buf, size_t buflen)
{ {
if(!FormatMessageA((FORMAT_MESSAGE_FROM_SYSTEM | 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)) LANG_NEUTRAL, buf, (DWORD)buflen, NULL))
msnprintf(buf, buflen, "Unknown error %d (%#x)", err, err); msnprintf(buf, buflen, "Unknown error %d (%#x)", err, err);
return buf; return buf;
@ -247,7 +247,7 @@ int wait_ms(int timeout_ms)
#if defined(MSDOS) #if defined(MSDOS)
delay(timeout_ms); delay(timeout_ms);
#elif defined(USE_WINSOCK) #elif defined(USE_WINSOCK)
Sleep(timeout_ms); Sleep((DWORD)timeout_ms);
#else #else
pending_ms = timeout_ms; pending_ms = timeout_ms;
initial_tv = tvnow(); initial_tv = tvnow();

View File

@ -368,7 +368,7 @@ UNITTEST_START
happens */ happens */
for(byte = 1 ; byte < 255; byte += 17) { for(byte = 1 ; byte < 255; byte += 17) {
for(i = 0; i < 45; i++) { for(i = 0; i < 45; i++) {
char backup = cert[i]; unsigned char backup = cert[i];
cert[i] = (unsigned char) (byte & 0xff); cert[i] = (unsigned char) (byte & 0xff);
(void) Curl_extract_certinfo(data, 0, beg, end); (void) Curl_extract_certinfo(data, 0, beg, end);
cert[i] = backup; cert[i] = backup;