[OpenAFS-devel] Re: Making supergroups default to on

[chas williams - CONTRACTOR]

On Fri, 28 Dec 2012 14:20:17 -0500
Andrew Deason <adeason@sinenomine.net> wrote:

> From what I recall, the code relies heavily on aliasing that gcc warns
> about (that is, casting pointers to different structures) and does some
> strange things with pointer bits. While the warnings themselves don't
> necessarily mean that the code is 'bad', getting rid of the warnings
> properly would probably involve the desired improvements.

I can't comment about that part of the code but quite a bit of
the supergroups code could benefit from refactoring.   this does
make maintaining the code a bit of a concern since changes to the
non-supergroups portion of the code wouldnt always fix a bit in the
supergroups code.

example below.  IsAMemberOf() is really IsAMemberOfSG() with depth=1 but
the supergroup code is a copy of IsAMemberOf() with supergroups patches.


diff --git a/src/ptserver/utils.c b/src/ptserver/utils.c
index 732f585..d8760b8 100644
--- a/src/ptserver/utils.c
+++ b/src/ptserver/utils.c
@@ -19,11 +19,9 @@
 #include "ptserver.h"
 #include "pterror.h"
 
-#if defined(SUPERGROUPS)
 extern afs_int32 depthsg;
 afs_int32 IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid,
 			afs_int32 depth);
-#endif
 
 static afs_int32
 IDHash(afs_int32 x)
@@ -801,6 +799,7 @@ OwnerOf(struct ubik_trans *at, afs_int32 gid)
 }
 
 
+#ifdef notdef
 afs_int32
 IsAMemberOf(struct ubik_trans *at, afs_int32 aid, afs_int32 gid)
 {
@@ -860,9 +859,18 @@ IsAMemberOf(struct ubik_trans *at, afs_int32 aid, afs_int32 gid)
     return 0;			/* actually, should never get here */
 #endif
 }
+#endif
 
-
+afs_int32
+IsAMemberOf(struct ubik_trans *at, afs_int32 aid, afs_int32 gid)
+{
 #if defined(SUPERGROUPS)
+    return IsAMemberOfSG(at, aid, gid, depthsg);
+#else
+    return IsAMemberOfSG(at, aid, gid, 1);
+#endif
+}
+
 afs_int32
 IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid, afs_int32 depth)
 {
@@ -875,6 +883,18 @@ IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid, afs_int32 dep
 
     if (depth < 1)
 	return 0;
+
+    /* special case anyuser and authuser */
+    if (gid == ANYUSERID)
+        return 1;
+    if (gid == AUTHUSERID && aid != ANONYMOUSID)
+        return 1;
+    /* check -localauth case */
+    if (gid == SYSADMINID && aid == SYSADMINID)
+        return 1;
+    if ((gid == 0) || (aid == 0))
+        return 0;
+
     loc = FindByID(at, gid);
     if (!loc)
 	return 0;
@@ -890,10 +910,12 @@ IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid, afs_int32 dep
 	    return 0;
 	if (gid == aid)
 	    return 1;
+/*
 	if (gid == ANYUSERID)
 	    return 1;
 	if (gid == AUTHUSERID && aid != ANONYMOUSID)
 	    return 1;
+*/
 	if (gid < 0) {
 #ifndef AFS_PTHREAD_ENV
 	    IOMGR_Poll();
@@ -915,10 +937,12 @@ IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid, afs_int32 dep
 		    return 0;
 		if (gid == aid)
 		    return 1;
+/*
 		if (gid == ANYUSERID)
 		    return 1;
 		if (gid == AUTHUSERID && aid != ANONYMOUSID)
 		    return 1;
+*/
 		if (gid < 0) {
 #ifndef AFS_PTHREAD_ENV
 		    IOMGR_Poll();
@@ -932,4 +956,3 @@ IsAMemberOfSG(struct ubik_trans *at, afs_int32 aid, afs_int32 gid, afs_int32 dep
     }
     return 0;			/* actually, should never get here */
 }
-#endif /* SUPERGROUPS */