This change has no user-visible effect so no NEWS entry is required.

Please include a news entry.

We typically only skip news if the change is a trivial internal change or change is already covered by a previous PR. Non-trivial commits get a news entry so other core devs, release manager, distributors, and users have an easier way to locate a problem when a change introduces a regression.

@tiran Done (initially I've thought this change is invisible to users so skipped it).

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 phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

if (groups_size >= 0)

setgroups(0, NULL) is valid.

For some reason,setgroups(0, NULL) causesDoctest,Check if generated files are up to date, and SSL workflows to fail child processes withPermissionError: [Errno 1] Operation not permitted exception:

Run ./python Lib/test/ssltests.pyTraceback (most recent call last):OpenSSL 1.1.1n  15 Mar 2022  File "/home/runner/work/cpython/cpython/Lib/test/ssltests.py", line 37, in <module>    run_regrtests(*sys.argv[1:])  File "/home/runner/work/cpython/cpython/Lib/test/ssltests.py", line 33, in run_regrtests    result = subprocess.call(args)             ^^^^^^^^^^^^^^^^^^^^^  File "/home/runner/work/cpython/cpython/Lib/subprocess.py", line 378, in call    with Popen(*popenargs, **kwargs) as p:         ^^^^^^^^^^^^^^^^^^^^^^^^^^^  File "/home/runner/work/cpython/cpython/Lib/subprocess.py", line 1011, in __init__    self._execute_child(args, executable, preexec_fn, close_fds,  File "/home/runner/work/cpython/cpython/Lib/subprocess.py", line 1888, in _execute_child    raise child_exception_type(errno_num, err_msg, err_filename)PermissionError: [Errno 1] Operation not permittedError: Process completed with exit code 1.

However, this is fixed by excluding a zero from the comparison:if (groups_size > 0). I think we may do it this way becausesetgroups(0, NULL) is a no-op inchild_exec (a positive returned value is dropped anyway):

If size is zero, list is not modified, but the total number of supplementary group IDs for the process is returned.

--https://linux.die.net/man/2/setgroups

For Bedevere: I have made the requested changes; please review again. For correct jump from Mentioned to this message instead of the following Bedevere's one:@gpshead.

Thanks for making the requested changes!

@gpshead: please review the changes made to this pull request.

@gpshead@tiran could you reconsider this PR please? This is a prerequirement forgh-94519, created to get rid of uninitialized variables.

We are currently busy with 3.11 release. It may take until 3.11.0 final before we come back to this patch. It's definitely on the list for 3.12.

Todo: replacegroups_size/groups parameter names withextra_group_size/extra_groups across the whole file to avoid confusion of passers-by. It's necessary to mitigate a misleading name ofsetgroups that supposes replacement of not supplimentary group affinities, but all ones. The mislead results in seeing the omission ofsetgroups call forgroups_size == 0 as a bug (like we should reset the inherited affinities but do nothing instead).

I'll do it in a separate PR because both this PR andgh-94519 are already big enough to clobber them with extra details.

@tiran could you review and possibly merge this PR please? It's an implementation of your proposal#94519 (comment) created as a separate PR to preserve diffs of both PRs readable.

Thank you for merging, Gregory!