-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
gh-121427: Fixed type errors triggered when compiling with MSYS2/UCRT64
#126355
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
| // On Windows, timeval.tv_sec type is long | ||
| res2 = _PyTime_AsCLong(tv_sec, &tv->tv_sec); | ||
| #elif _UCRT | ||
| res2 = _PyTime_AsTime_t(tv_sec, (time_t *)&tv->tv_sec); |
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.
This is not the correct fix. From the error message "expected 'time_t *' {aka 'long long int *'} but argument is of type 'long int *'" it can be seen that tv->tv_sec has type long int which is different from long long int.
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.
Actually my initial thought was to start with struct timeval, but maybe that's not appropriate :(
Edit
(I actually think casting to the target type might be better than this.)
Edit2
Suddenly I remembered that struct timeval seems to be defined in the system header file.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Hmmm.... Maybe this problem cannot be solved fundamentally. Unless we start from the function signature. At least I think that casting might be the solution. The author gave a thumbs up as feedback :) |
|
Of course, it can be solved. You should just understand what this code do and what goes wrong. Find a function similar to |
|
In fact, what I'm worried about is that although forced conversion can solve the problem, what if the number value is cut off. (Maybe not) |
|
No, it does not solve the problem. It creates a possible security issue (buffer overflow). |
|
@rruuaanng: please do not create PRs for half-baked solutions; |
Actually, it only requires preprocessing the parameter passing. I hope it won't cause data type truncation.
cc @vstinner