Re: [PATCH] Many groups patch.

[Tim Hockin <thockin@hockin.org>]

On Wed, Oct 01, 2003 at 12:22:03PM -0700, Linus Torvalds wrote:
> Augh. It also makes code even uglier than it used to be:
> 
> 	...
> 
> 	+               u16 group;
> 	+               if (copy_from_user(&group, grouplist+i, sizeof(group)))
> 	+                       return  -EFAULT;

I can change it to do a copy_from_user one block at a time, if you prefer...

> 	if (nr > TASK_SIZE / sizeof(group))
> 		return -EFAULT;
> 	if (!access_ok(grouplist, nr*sizeof(group))
> 		return -EFAULT;
> 	...
> 
> 		if (__get_user(group, grouplist + i))
> 			return -EFAULT;
> 	...

Or change it to this, which is the same 1-gid-at-a-time copy.  This code is
definitely SIMPLER than the 1-block-at-a-time copy.  I'll go with that.

> which really is so common that it _really_ should be in kernel/uid16.c
> (or, actually create a new kernel/gid16.c file) rather than copied 
> (incorrectly) to a lot of architectures. Then things like the above can be 
> done right once, rather than merging this that does the nasty thing over 
> and over.

I'd love to put it in uid16.c, but uid16.c is not used by the 64-bit
architectures.  I remember proposing a simple fix at one point and being
shot down.  I'd love to fix that up, but it's a separate patch.  If I
fix it up as suggested above, will you take it with the promise that I'll
find some way to do uid16 properly for the 64-bit arches and clean it up in
a followup patch?

> Sorry to just complain all the time,

I'm just glad it's getting attention - I'm dying to take this off my todo
list.

Tim