Re: [PATCH] Many groups patch.

[Rusty Russell <rusty@rustcorp.com.au>]

In message <20031001184610.GA25716@hockin.org> you write:
> Because groups are stored in a 2-D array, the GRP_AT()
> macro was added to allow simple 1-D style indexing.

I respectfully disagree with this approach.  I don't understand why
Linus would think that 4 lines of code to fallback to vmalloc is
"ugly", when doing your own array of pages means you have to change
all the iterators and do magic when copying to and from userspace,
since it's no longer a simple array.

Tim and I have had this discussion, but I would benifit from Linus'
wisdom here.

> Groups are sorted and b-searched for efficiency.

That's where I feel Linus' comment about catering to stupidity comes
in 8)

> This patch modifies /proc/pid/status to only display the first 32 groups.

Ouch, good catch, but this is broken in both our patches 8( I think we
should bump the refcount before reading it to avoid accessing freed
memory.  Now, they have to be root to access that anyway, but it just
makes me uncomfortable.

> This patch removes the NGROUPS define from all architectures as well as
> NGROUPS_MAX.

This will break glibc build (trivial to fix), but I agree: leaving
NGROUPS_MAX is too likely to cause people to use it inside the kernel
like intermezzo does.

> This patch changes the security API to check a struct group_info, rather
> than an array of gid_t.

This is why I prefer the "fall back to vmalloc" approach.

> This patch totally horks Intermezzo.

That's a feature 8)

Thanks,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.