Skip to content

uucore: correctly truncate response if getgroups shrinks #6978

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

Merged
merged 2 commits into from
Dec 19, 2024
Merged

uucore: correctly truncate response if getgroups shrinks #6978

merged 2 commits into from
Dec 19, 2024

Conversation

santtu
Copy link
Contributor

@santtu santtu commented Dec 19, 2024

The code above the line with truncate handles the case if res is larger than ngroups, but res < ngroups is also a possibility, which this line attempts to address but actually does not.

The original code resizes to ngroups which is a no-op (given that groups is already ngroups size). The correct target for re-sizing the groups is the result from the last getgroups, i.e., res.

The code above this line handles the case if `res` is
larger than `ngroups`, but `res < ngroups` is also a possibility,
which this line attempts to address but actually does not.

The original code resizes to `ngroups` which is a no-op (given
that `groups` is already `ngroups` size). The correct target for
re-sizing the `groups` is the result from the last `getgroups`,
i.e., `res`.
Copy link

GNU testsuite comparison:

GNU test failed: tests/misc/usage_vs_getopt. tests/misc/usage_vs_getopt is passing on 'main'. Maybe you have to rebase?

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@@ -83,13 +83,14 @@ pub fn get_groups() -> IOResult<Vec<gid_t>> {
if res == -1 {
let err = IOError::last_os_error();
if err.raw_os_error() == Some(libc::EINVAL) {
// Number of groups changed, retry
// Number of groups has increased, retry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure it is always increased ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POSIX.1-1988 on getgroups return value:

[EINVAL]
The gidsetsize argument is non-zero and less than the number of group IDs that would have been returned.

Since ngroups is based on earlier getgroups return value, the only case where EINVAL is returned when the indicated size (ngroups parameter for getgroups) is less than what the return list would be. If the reserved space is larger than the actual list, then res != -1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, cool, thanks :)

@sylvestre sylvestre merged commit 2ae914b into uutils:main Dec 19, 2024
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants