On Fri, Jan 30, 2004 at 03:08:19PM -0800, Andrew Morton wrote:

I think this is right - the NFSEXP_ALLSQUASH case appears to be clearing
all groups.  When this settles down we need to run it all by Neil.

Do we need to handle the return value from set_current_groups(), or should
that guy be simply returning void?

set_current_groups() can fail if security_task_setgroups() fails.

+	struct group_info *group_info = NULL;

Why init to NULL?

+	ngroups = 0;
+	if (!(exp->ex_flags & NFSEXP_ALLSQUASH)) {
+		for (i = 0; i < SVC_CRED_NGROUPS; i++) {
+			if (cred->cr_groups[i])
+				ngroups++;
+		}
+	}

I though of doing this, but passed in favor of simplicity of patch :)

The original made a specific point of doing
	gid_t group = cred->cr_groups[i];
	if (group == (gid_t) NOGROUP)
		break;

So the count loop should probably be
	ngroups = 0;
	if (!(exp->ex_flags & NFSEXP_ALLSQUASH)) {
		for (i = 0; i < SVC_CRED_NGROUPS; i++) {
			gid_t group = cred->cr_groups[i];
			if (group == (gid_t) NOGROUP)
				break;
			ngroups++;
		}
	}
So that we don't assume anything about NOGROUP.

+	return ret;

The caller in fs/nfsd/nfsfh.c still needs to check the return value and do
something with it, or all this is just dumb.

-- 
Tim Hockin
Sun Microsystems, Linux Software Engineering
thockin@sun.com
All opinions are my own, not Sun's
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>