-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-127081: use getlogin_r
if available
#132751
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
The getlogin function is not thread-safe: replace with getlogin_r where available. Note that this function is untested (unit test is skipped with a note it caused CI failures as behaviour differs between NIX environments).
getlogin_r
if available
…at order) if available, falling back to a hard-coded maximum login name length of 255 if neither is available.
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 should get a test case.
The specific change or The existing (unconditionally skipped) test could be re-enabled, but it doesn't test very much at all, and presumably will still fail on some CI systems. Perhaps we could expand it to get the "correct" result to check against via a shell command? Then if/when it fails on specific platforms either skip or special-case the test for them? Testing the specific change: I can add a multi-threaded test, but it would ideally be conditional on having |
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 675342c 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132751%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
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 LGTM assuming the build bots pass
Buildbot failures look unrelated to me. |
Thanks @duaneg for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Thanks @duaneg for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
The `getlogin` function is not thread-safe: replace with `getlogin_r` where available. (cherry picked from commit 1ffe913) Co-authored-by: Duane Griffin <duaneg@dghda.com>
Sorry, @duaneg and @colesbury, I could not cleanly backport this to
|
GH-135097 is a backport of this pull request to the 3.14 branch. |
The `getlogin` function is not thread-safe: replace with `getlogin_r` where available. (cherry picked from commit 1ffe913) Co-authored-by: Duane Griffin <duaneg@dghda.com>
GH-135098 is a backport of this pull request to the 3.13 branch. |
|
The getlogin function is not thread-safe: replace with getlogin_r where available.
Note that this function is untested (unit test is skipped with a note it caused CI failures as behaviour differs between NIX environments).