build: enable -Wsign-conversion warnings and fix/silence them (OpenSSF)
#13489
Conversation
vszakats
changed the title
-Wsign-conversion warnings-Wsign-conversion warnings
vszakats
force-pushed
the
signwarn-10
branch
2 times, most recently
from
1faf8eb
to
b73157c
Compare
|
PR now passes all CI builds and tests. It did help finding small issues, but I haven't investigated all possible ones, only the trivial ones. About half of the patch is addressing To my eyes the resulting code better expresses what's happening and more clearly describes There is no exact number for the warnings fixed. There were about 450 in the first round and Of these, around 140 were silenced with the It's possible that warnings are still present in code not compiled in CI and/or build combinations Next steps?:
|
vszakats
changed the title
-Wsign-conversion warnings-Wsign-conversion warnings and fix/silence them all
vszakats
changed the title
-Wsign-conversion warnings and fix/silence them all-Wsign-conversion warnings and fix/silence them
vszakats
force-pushed
the
signwarn-10
branch
2 times, most recently
from
7364f36
to
98c57d8
Compare
vszakats
changed the title
-Wsign-conversion warnings and fix/silence them-Wsign-conversion warnings and fix/silence them (OpenSSF)
vszakats
force-pushed
the
signwarn-10
branch
2 times, most recently
from
8bf863f
to
4dd290d
Compare
|
Separate PRs for the major curl components:
The above are the same sub-commits as here, except two controlling the warning option, which is unique to this one. |
To match the type used in 'set.happy_eyeballs_timeout'. Ref: #13489
There was a problem hiding this comment.
I just think that adding typecasts in hundreds of new places is not a big win. They just silence the warnings with code that is a tad bit harder to read, the same logic is still there.
The improvements done by changing variable types and prototypes are the interesting ones I think. A typecast is always a bit of a "I surrender" sign.
temp:
```
C:/projects/curl/lib/vtls/x509asn1.c:961:12: error: cast from function call of type 'CURLcode' to non-matching type 'int' [-Werror=bad-function-cast]
961 | return (int)do_pubkey_field(data, certnum, "ecPublicKey", pubkey);
| ^
```
Ref: https://ci.appveyor.com/project/curlorg/curl/builds/49704256/job/hcietrd5e7qbu73c#L326
```
D:/a/curl/curl/tests/unit/unit1604.c:322:37: error: implicit conversion changes signedness: 'SANITIZEcode' to 'int' [-Werror,-Wsign-conversion]
322 | received_ccstr = getcurlcodestr(res);
| ~~~~~~~~~~~~~~ ^~~
D:/a/curl/curl/tests/unit/unit1604.c:324:45: error: implicit conversion changes signedness: 'SANITIZEcode' to 'int' [-Werror,-Wsign-conversion]
324 | expected_ccstr = getcurlcodestr(data[i].expected_result);
| ~~~~~~~~~~~~~~ ~~~~~~~~^~~~~~~~~~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/9340164046/job/25705385260#step:13:795
```
../../lib/krb5.c:633:60: error: implicit conversion changes signedness: 'enum protection_level' to 'int' [-Werror,-Wsign-conversion]
bytes = conn->mech->encode(conn->app_data, from, length, prot_level,
~~~~ ^~~~~~~~~~
../../lib/krb5.c:727:36: error: implicit conversion changes signedness: 'enum protection_level' to 'int' [-Werror,-Wsign-conversion]
level, conn);
^~~~~
```
Ref: https://github.com/curl/curl/actions/runs/9340164051/job/25705383216?pr=13489#step:3:1411
```
../../../tests/libtest/sethostname.c: In function 'gethostname':
../../../tests/libtest/sethostname.c:34:35: error: conversion to 'size_t' {aka 'long unsigned int'} from 'int' may change the sign of the result [-Werror=sign-conversion]
34 | strncpy(name, force_hostname, namelen);
| ^~~~~~~
```
Ref: https://github.com/curl/curl/actions/runs/9340164051/job/25705383801?pr=13489#step:3:8501
This reverts commit a67a427.
|
|
-Wsign-conversionwarnings bysize_tandcurl_off_tcasts.-Wsign-conversionwarnings (OpenSSF).-Wsign-conversionan error (OpenSSF).Related fixes, including type changes or casts in interface boundaries:
-Wsign-conversionwarning forFD_SET()/FD_ISSET()macros. → build: silence-Wsign-conversioninFD_SET()/FD_ISSET()#13896These trigger on some systems due to the macros' implementation.
fix signedness in some→ tests: make the unit test result typeprintfmasks.CURLcode#13600 lib: tidy up types and casts #13862tests: switch unit tests to return→ tests: make the unit test result typeCURLcode.CURLcode#13600mingw declares→ lib: tidy up types and casts #13862sin_family/sin6_familyasshort.openssl: fix→ lib/v*: tidy up types and casts #13622ctx_option_ttype for openssl 1.1.xsectransp: fix→ lib/v*: tidy up types and casts #13622CURLcode/OSStatusmix-up.tool_parsecfg: fix→ src: tidy up types, add necessary casts #13614ParameterErrormix-up.fix assert in→ hash: change 'slots' to size_t from int #13502 lib: bump hash sizes toCurl_hash_add.size_t#13601Follow-up to 3829759 #12489
Closes #13489