-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
lib: silence -Wsign-conversion
, tidy-ups, fixes
#13481
Closed
+443
−367
Closed
Changes from all commits
Commits
Show all changes
77 commits
Select commit
Hold shift + click to select a range
0ad8b49
lib altsvc
vszakats 3c270a3
lib altsvc switch prio to unsigned int
vszakats d9231e3
lib asyn-ares
vszakats 2511495
lib asyn-thread win32
vszakats f7530f5
lib bufq
vszakats 8c17732
lib cf-h2-proxy
vszakats b94206b
lib cf-socket
vszakats 9606299
lib c-hyper
vszakats 3391e21
lib cfilters
vszakats 29321ae
lib content_encoding
vszakats 8d03ece
lib cookie
vszakats e63dbe5
lib curl_addrinfo mingw
vszakats 8153c63
lib curl_fnmatch
vszakats ae40d07
lib curl_gethostname
vszakats a8317f7
lib curl_gssapi
vszakats 82db7d7
lib curl_multibyte
vszakats 26af888
lib curl_ntlm_core
vszakats 0390180
lib curl_path
vszakats de4f8f9
lib curl_sspi
vszakats fa41c91
lib curl_trc
vszakats d0ce612
lib dynhds
vszakats d2c89b1
lib doh
vszakats 22e9405
lib nonblock
vszakats 4245995
lib inet_ntop
vszakats 01ac885
lib inet_ntop.h win32
vszakats beff682
lib inet_pton
vszakats e9a8486
lib easy
vszakats bb3c232
lib escape
vszakats f98e79a
lib file
vszakats 837b441
lib smtp
vszakats 2700e8e
lib setopt
vszakats 14e2f87
lib select
vszakats af2801e
lib select FD_SET/FD_ISSET workaround
vszakats efedcd4
lib request
vszakats 5904160
lib pingpong
vszakats 81c4a27
lib formdata
vszakats dc23139
lib ftp
vszakats e1b531a
lib getinfo
vszakats 8fb92aa
lib gopher
vszakats 796dd44
lib hash
vszakats e2b566a
lib headers
vszakats 3e7cb8c
lib hostip
vszakats 44cd7dc
lib http
vszakats 0c58246
lib http1
vszakats 3c557c7
lib http2
vszakats f25aa4c
lib http_aws_sigv4
vszakats d7002be
lib http_chunks
vszakats ac91fea
lib http_digest
vszakats cc4018e
lib imap
vszakats 5dc7ca8
lib timeval
vszakats 7ac27b2
lib transfer
vszakats 1223fe7
lib url
vszakats cf0c465
lib urlapi
vszakats 34d8b0d
lib krb5
vszakats 7286832
lib openldap
vszakats 54807a8
lib ldap
vszakats c385829
lib pop3
vszakats 023d545
lib rand
vszakats 30dd5d1
lib rtsp
vszakats 218b496
lib noproxy
vszakats 1855987
lib sendf
vszakats 89cbb56
lib smb
vszakats 37276b0
lib memdebug.h
vszakats 9f73afa
lib mime
vszakats e6d0ff9
lib mqtt
vszakats e619b52
lib multi
vszakats bba4d94
lib multi suppress sign-conversion for FD_SET
vszakats 263c630
lib socketpair
vszakats deaf629
lib socks
vszakats 1d7dbca
lib socks_gssapi
vszakats a105fc9
lib socks_sspi
vszakats cdeb359
lib strerror
vszakats 0687bbe
lib telnet
vszakats 755d7dd
lib tftp
vszakats 28dde17
lib tftp win32
vszakats fbb7e11
lib ws
vszakats 3c39ee1
lib version
vszakats File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this kind of typecast is perhaps the biggest sign that we should not use this warning option. The diff between two
char *
should be possible to store in asize_t
without a typecast.Or put another way: do these typecasts really help? They will make writing new code a little more awkward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It pinpoints all the places where the result might be negative,
but the code assumes positive. For the
strchr()
pattern and manualversions of it, this isn't really useful, but may be in the rest.
I assumed that all existing code is correct, but this isn't readily obvious
while reviewing the 442 cases.
A cast adds a signal that sign conversions were consious decisions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code does not assume this, it knows. The logic is quite easy to follow.
In this and similar cases, this typecast does not add anything at all to a reader. In fact, it is mostly confusing to me since a pointer-delta should already be possible to store in a `size_t' just fine. Should it not?
Have you detected and corrected any actual errors with this exercise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some, that were obvious and low-hanging, yes.
But, I try not to touch the actual logic, first because it'd take a very long time to explore each rabbit hole, and second it'd make the patch confusing IMO. Also, it's highly likely the code is correct unless proven otherwise.
What I find that it's difficult to deduct from local contexts that the code is handling the implicit sign conversions conciously.
Am I mistaken that e.g. a 100 - 1000 pointer delta would result in an underflow and make the result a very high number when handled as an unsigned
size_t
?The other thing driving this is that this is one the few warnings actually recommended by OpenSSF. Assuming they know what they are saying, this might actually be useful, though of course there is nothing saying we must follow their recommendations.