bpo-40014: Fix os.getgrouplist() failure on macOS with many groups #19118
Conversation
|
@ambv @ned-deily: Would you mind to review this fix? |
Modules/posixmodule.c
Outdated
There was a problem hiding this comment.
Just question:
I 've understood the situation.
Is there any reason not to try with INT_MAX for the last trying?
if (ngroups == INT_MAX) {
return PyErr_NoMemory();
} else if (ngroups > INT_MAX / 2) {
ngroups = INT_MAX;
} else {
ngroups *= 2;
}There was a problem hiding this comment.
If (ngroups > INT_MAX / 2) is false, ngroups *= 2 would overflow: ngroups type is int.
} else if (ngroups > INT_MAX / 2) {
ngroups = INT_MAX;
I don't think that it's worth it to bother with attempting that. I don't think that any user has more than 1073741823 groups. Such user is going to get larger issues than that.
There was a problem hiding this comment.
Okay got it :) Sounds reasonable.
ned-deily
left a comment
ned-deily
left a comment
There was a problem hiding this comment.
The change itself looks good, thanks. But, as noted on the bpo, with further testing, it seems this is not a new issue with 10.15: it also fails on 10.14 and 10.13. I didn't test any older systems. So the comments and the PR title should be changed to remove the version references and just refer to something like "some versions of macOS".
|
When you're done making the requested changes, leave the comment: |
On macOS, getgrouplist() returns a non-zero value without setting errno if the group list is too small. Double the list size and call it again in this case.
Sorry, I missed that comment. I rewrote my PR to remove all Darwin and macOS versions. I just wrote "on macOS", it should be enough ;-) I added the bpo number anyway, for curious people wanting the further details ;-) |
ned-deily
changed the title
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8. |
On macOS, getgrouplist() returns a non-zero value without setting errno if the group list is too small. Double the list size and call it again in this case. (cherry picked from commit 8ec7370) Co-authored-by: Victor Stinner <[email protected]>
|
GH-19123 is a backport of this pull request to the 3.8 branch. |
|
GH-19124 is a backport of this pull request to the 3.7 branch. |
On macOS, getgrouplist() returns a non-zero value without setting errno if the group list is too small. Double the list size and call it again in this case. (cherry picked from commit 8ec7370) Co-authored-by: Victor Stinner <[email protected]>
On macOS, getgrouplist() returns a non-zero value without setting errno if the group list is too small. Double the list size and call it again in this case. (cherry picked from commit 8ec7370) Co-authored-by: Victor Stinner <[email protected]>
On macOS, getgrouplist() returns a non-zero value without setting errno if the group list is too small. Double the list size and call it again in this case. (cherry picked from commit 8ec7370) Co-authored-by: Victor Stinner <[email protected]>
6 participants
On Darwin 19.3 (macOS 10.15 Catalina), getgrouplist() returns a
non-zero value without setting errno if the group list is too small.
Double the list size and call it again in this case.
https://bugs.python.org/issue40014