quota_group_type has some rather contorted logic that's
been around since 2005.
In the (!name) case, if any of the 3 calls setting up ngroups fails,
we fall back to using just one group.
However, if it's the getgroups() call that fails, we overwrite
the allocated gid ptr with &gid, thus leaking that allocated
memory. Worse, we set "dofree" to 1, so will free non-allocated
local var gid. And that last else case is redundant; if we get there,
gids is guaranteed to be non-null.
Refactor it a bit to be more clear (I hope) and correct.
Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
---
quota/quota.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/quota/quota.c b/quota/quota.c
index 7e52ad2..367da8c 100644
--- a/quota/quota.c
+++ b/quota/quota.c
@@ -289,15 +289,19 @@ quota_group_type(
}
gids = &gid;
ngroups = 1;
- } else if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
- ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
- ((ngroups = getgroups(ngroups, gids)) < 0)) {
- dofree = (gids != NULL);
- gid = getgid();
- gids = &gid;
- ngroups = 1;
} else {
- dofree = (gids != NULL);
+ if ( ((ngroups = sysconf(_SC_NGROUPS_MAX)) < 0) ||
+ ((gids = malloc(ngroups * sizeof(gid_t))) == NULL) ||
+ ((ngroups = getgroups(ngroups, gids)) < 0)) {
+ /* something failed. Fall back to 1 group */
+ free(gids);
+ gid = getgid();
+ gids = &gid;
+ ngroups = 1;
+ } else {
+ /* It all worked, and we allocated memory */
+ dofree = 1;
+ }
}
for (i = 0; i < ngroups; i++, name = NULL) {
--
1.7.1
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
