-
-
Notifications
You must be signed in to change notification settings - Fork 7k
multi: make max_total_* members size_t #19618
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
Conversation
|
This one needs a rebase! |
Check size_t conversion on setting these members via CURLMIPT_*. Use members without casting.
3df3256 to
96515fd
Compare
lib/curlx/warnless.c
Outdated
| *puznum = 0; | ||
| return FALSE; | ||
| } | ||
| #if SIZEOF_LONG > SIZEOF_SIZE_T |
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.
Does such a platform actually exist?
Long is always at least 32 bits. Often 64 bits. Is there a platform where long is 64 bits where size_t is not?
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 do not know. is it forbidden?
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'm thinking:
1.if it can't happen we don't need to check for it here
2.if it actually can happen, I bet there are other places that would need fixing
3.Maybe we should have a single spot in curl_setup.h or somewhere that just #error on this condition to make us certain it is forbidden in our code?
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.
As I understand the C language standard, there is no relation defined between long and size_t. So, it seems possible but unlikely.
A general catch in curl_setup.h might be good, especially given that long appears in our API a lot.
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.
Moved the check info curl_setup.h with an #error.
Check size_t conversion on setting these members via CURLMIPT_*. Use members without casting.