Re: setgroups

Re: setgroups

[Rob Browning]

(Followups set to guile-devel -- I think...)

prj@po.cwru.edu (Paul Jarc) writes:

> Not a bug, really, but a significant (to me, anyway :) ) missing
> feature.  Can we have setgroups?

I saw this and was about to just add it, but then realized I didn't
know what our policies were wrt to functions that might or might not
exist at runtime...

If I'm not mistaken, setgroups might or might not be available on a
given system.  We can test for it in configure and optionally define
it in posix.c (or maybe elsewhere since it's not posix...), but then
what?

Given our current uncertainty about compilation, it seems like

  (if (defined? 'setgroups) ...)

might be a bad idea for the long term, so how would we want to handle
this, via (provided? 'setgroups)?  i.e. is provided? supposed to be
used for things at that fine a granularity?  Also, it'd be nice if we
could use something that could in theory be optimized away like this
(though we probably wouldn't want this exactly...):

  (define-constant *have-setgroups?* ...)
  ...

  (if *have-setgroups?* ...)

etc.

Thanks

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Rob Browning <rlb@defaultvalue.org> wrote:
> (Followups set to guile-devel -- I think...)

Oops, right.

> If I'm not mistaken, setgroups might or might not be available on a
> given system.  We can test for it in configure and optionally define
> it in posix.c (or maybe elsewhere since it's not posix...), but then
> what?

Is the situation somehow different from that for getgroups?  I was
just about to (try to) make a patch that would add setgroups similarly
to how getgroups is done.


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Marius Vollmer]

prj@po.cwru.edu (Paul Jarc) writes:

> > If I'm not mistaken, setgroups might or might not be available on a
> > given system.  We can test for it in configure and optionally define
> > it in posix.c (or maybe elsewhere since it's not posix...), but then
> > what?
> 
> Is the situation somehow different from that for getgroups?  I was
> just about to (try to) make a patch that would add setgroups similarly
> to how getgroups is done.

Yes, I think we should handle setgroups like the other optional
functions, even if there are better ways.  Consistency is more
important here.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Ok, here's a patch for setgroups.  It's my first time writing Guile C
code, so someone should check it carefully.  I used a vector argument
just because that's what getgroups returns.

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.213
diff -u -r1.213 configure.in
--- configure.in	7 Apr 2003 17:31:02 -0000	1.213
+++ configure.in	17 Apr 2003 23:11:05 -0000
@@ -583,7 +583,7 @@
     [Define if the system supports Unix-domain (file-domain) sockets.])
 fi
 
-AC_CHECK_FUNCS(socketpair getgroups setpwent pause tzset)
+AC_CHECK_FUNCS(socketpair getgroups setgroups setpwent pause tzset)
 
 AC_CHECK_FUNCS(sethostent   gethostent   endhostent   dnl
                setnetent    getnetent    endnetent    dnl
Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.112
diff -u -r1.112 posix.c
--- libguile/posix.c	5 Apr 2003 19:10:22 -0000	1.112
+++ libguile/posix.c	17 Apr 2003 23:11:05 -0000
@@ -228,6 +228,36 @@
 #undef FUNC_NAME  
 #endif
 
+#ifdef HAVE_SETGROUPS
+SCM_DEFINE (scm_setgroups, "setgroups", 1, 0, 0,
+            (SCM group_vec),
+            "Set the supplementary group IDs to those found in the vector\n"
+            "argument.")
+#define FUNC_NAME s_scm_setgroups
+{
+  size_t ngroups;
+  size_t size;
+  size_t i;
+  int result;
+  GETGROUPS_T *groups;
+
+  SCM_VALIDATE_VECTOR (1, group_vec);
+
+  ngroups = SCM_VECTOR_LENGTH (group_vec);
+  size = ngroups * sizeof (GETGROUPS_T);
+  groups = scm_malloc (size);
+  for(i = ngroups; i >= 0; i--)
+    groups [i] = SCM_INUM (SCM_VECTOR_REF (group_vec, i));
+
+  result = setgroups (ngroups, groups);
+  free (groups);
+  if (result < 0)
+    SCM_SYSERROR;
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+#endif
+
 #ifdef HAVE_GETPWENT
 SCM_DEFINE (scm_getpwuid, "getpw", 0, 1, 0,
             (SCM user),


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Rob Browning]

prj@po.cwru.edu (Paul Jarc) writes:

> Is the situation somehow different from that for getgroups?  I was
> just about to (try to) make a patch that would add setgroups similarly
> to how getgroups is done.

My fault -- I failed to see the use of HAVE_GETGROUPS in posix.c --
dunno how I overlooked that.

My comment about it being different was just based on this from the
manpages:

  CONFORMING TO
       SVr4, SVID (issue 4 only;  these  calls  were  not  present  in  SVr3),
       X/OPEN, 4.3BSD.  The getgroups function is in POSIX.1.  Since setgroups
       requires privilege, it is not covered by POSIX.1.

Since I overlooked the HAVE_GETGROUPS, I guessed maybe we were just
presuming a POSIX system in posix.c.

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Rob Browning]

prj@po.cwru.edu (Paul Jarc) writes:

> Ok, here's a patch for setgroups.  It's my first time writing Guile C
> code, so someone should check it carefully.  I used a vector argument
> just because that's what getgroups returns.

That sounds about right...

> +  ngroups = SCM_VECTOR_LENGTH (group_vec);
> +  size = ngroups * sizeof (GETGROUPS_T);
> +  groups = scm_malloc (size);
> +  for(i = ngroups; i >= 0; i--)
> +    groups [i] = SCM_INUM (SCM_VECTOR_REF (group_vec, i));
> +
> +  result = setgroups (ngroups, groups);
> +  free (groups);

I might add a 

     scm_remember_upto_here_1 (group_vec);

here just to be safe, but that might be overkill, depending on what
SCM_VECTOR_REF does.

> +  if (result < 0)
> +    SCM_SYSERROR;
> +  return SCM_UNSPECIFIED;
> +}

Overall, it looks good to me.

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Marius Vollmer]

Rob Browning <rlb@defaultvalue.org> writes:

> I might add a 
> 
>      scm_remember_upto_here_1 (group_vec);
> 
> here just to be safe, but that might be overkill, depending on what
> SCM_VECTOR_REF does.

The scm_remember_upto_here is not necessary because SCM_VECTOR_REF
returns a SCM, which is properly protected by the conservative
scanning.  Had you used SCM_VELTS, then you would need the
scm_remember_upto_here since SCM_VELTS returns a 'naked' pointer to
the SCM array of the the vector and the conservative scanning knows
nothing about this (it only knows about SCMs).


However, just assuming that the vector holds inums is not OK, I'd say.
You need to use scm_num2ulong and then check that it fits in a gid_t.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Marius Vollmer <mvo@zagadka.de> wrote:
> However, just assuming that the vector holds inums is not OK, I'd say.
> You need to use scm_num2ulong and then check that it fits in a gid_t.

I agree that it would be good to check that it fits in gid_t, but
set[ug]id already fail do that, and I wouldn't know exactly what to do
in the case where it didn't fit.  So here's a version that's in line
with the way set[ug]id currently work.  If you explain how to throw
the appropriate error, I'll work on a patch to fix all such functions
I can find in posix.c.

There's another opportunity for overflow in this code; see the comment
below.

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.213
diff -u -r1.213 configure.in
--- configure.in	7 Apr 2003 17:31:02 -0000	1.213
+++ configure.in	18 Apr 2003 15:19:32 -0000
@@ -583,7 +583,7 @@
     [Define if the system supports Unix-domain (file-domain) sockets.])
 fi
 
-AC_CHECK_FUNCS(socketpair getgroups setpwent pause tzset)
+AC_CHECK_FUNCS(socketpair getgroups setgroups setpwent pause tzset)
 
 AC_CHECK_FUNCS(sethostent   gethostent   endhostent   dnl
                setnetent    getnetent    endnetent    dnl
Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.112
diff -u -r1.112 posix.c
--- libguile/posix.c	5 Apr 2003 19:10:22 -0000	1.112
+++ libguile/posix.c	18 Apr 2003 15:19:32 -0000
@@ -228,6 +228,41 @@
 #undef FUNC_NAME  
 #endif
 
+#ifdef HAVE_SETGROUPS
+SCM_DEFINE (scm_setgroups, "setgroups", 1, 0, 0,
+            (SCM group_vec),
+	    "Set the supplementary group IDs to those found in the vector\n"
+            "argument.")
+#define FUNC_NAME s_scm_setgroups
+{
+  size_t ngroups;
+  size_t size;
+  size_t i;
+  int result;
+  GETGROUPS_T *groups;
+
+  SCM_VALIDATE_VECTOR (0, group_vec);
+
+  ngroups = SCM_VECTOR_LENGTH (group_vec);
+  size = ngroups * sizeof (GETGROUPS_T);
+  /* XXX - if (ngroups / sizeof (GETGROUPS_T) != size) out-of-range */
+  groups = scm_malloc (size);
+  for(i = ngroups; i >= 0; i--)
+    {
+      SCM gid = SCM_VECTOR_REF (group_vec, i);
+      SCM_VALIDATE_INUM (0, gid);
+      groups [i] = SCM_INUM (gid);
+    }
+
+  result = setgroups (ngroups, groups);
+  free (groups);
+  if (result < 0)
+    SCM_SYSERROR;
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+#endif
+
 #ifdef HAVE_GETPWENT
 SCM_DEFINE (scm_getpwuid, "getpw", 0, 1, 0,
             (SCM user),


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Rob Browning]

Marius Vollmer <mvo@zagadka.de> writes:

> However, just assuming that the vector holds inums is not OK, I'd
> say.  You need to use scm_num2ulong and then check that it fits in a
> gid_t.

Ahh, right.  If there's nothing like GID_MAX, I guess we'll need to
compute the fit manually.

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Rob Browning <rlb@defaultvalue.org> wrote:
> If there's nothing like GID_MAX, I guess we'll need to compute the
> fit manually.

unsigned long lgid = SCM_INUM (foo);
GETGROUPS_T gid = lgid;
if (gid < 0 || lgid != (unsigned long) gid) overflow;

- Is SCM_INUM guaranteed to fit in unsigned long?  If not, what type
  can we use instead?
- If GETGROUPS_T is signed and "gid = lgid" overflows, the C standard
  says it's undefined behavior.  Are we worried about that, or are we
  confident that it'll just wrap on platforms we care about?


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Rob Browning]

prj@po.cwru.edu (Paul Jarc) writes:

> unsigned long lgid = SCM_INUM (foo);
> GETGROUPS_T gid = lgid;
> if (gid < 0 || lgid != (unsigned long) gid) overflow;
>
> - Is SCM_INUM guaranteed to fit in unsigned long?  If not, what type
>   can we use instead?

I believe scm_t_bits (there's also scm_t_signed_bits) may be what you
want.

> - If GETGROUPS_T is signed and "gid = lgid" overflows, the C standard
>   says it's undefined behavior.  Are we worried about that, or are we
>   confident that it'll just wrap on platforms we care about?

I wondered about signed vs unsigned gid_t too...

-- 
Rob Browning
rlb @defaultvalue.org, @linuxdevel.com, and @debian.org
Previously @cs.utexas.edu
GPG starting 2002-11-03 = 14DD 432F AE39 534D B592  F9A0 25C8 D377 8C7E 73A4


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Off-by-one bug in that last version.

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.213
diff -u -r1.213 configure.in
--- configure.in	7 Apr 2003 17:31:02 -0000	1.213
+++ configure.in	18 Apr 2003 17:37:56 -0000
@@ -583,7 +583,7 @@
     [Define if the system supports Unix-domain (file-domain) sockets.])
 fi
 
-AC_CHECK_FUNCS(socketpair getgroups setpwent pause tzset)
+AC_CHECK_FUNCS(socketpair getgroups setgroups setpwent pause tzset)
 
 AC_CHECK_FUNCS(sethostent   gethostent   endhostent   dnl
                setnetent    getnetent    endnetent    dnl
Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.112
diff -u -r1.112 posix.c
--- libguile/posix.c	5 Apr 2003 19:10:22 -0000	1.112
+++ libguile/posix.c	18 Apr 2003 17:37:57 -0000
@@ -228,6 +228,41 @@
 #undef FUNC_NAME  
 #endif
 
+#ifdef HAVE_SETGROUPS
+SCM_DEFINE (scm_setgroups, "setgroups", 1, 0, 0,
+            (SCM group_vec),
+	    "Set the supplementary group IDs to those found in the vector\n"
+            "argument.")
+#define FUNC_NAME s_scm_setgroups
+{
+  size_t ngroups;
+  size_t size;
+  size_t i;
+  int result;
+  GETGROUPS_T *groups;
+
+  SCM_VALIDATE_VECTOR (0, group_vec);
+
+  ngroups = SCM_VECTOR_LENGTH (group_vec);
+  size = ngroups * sizeof (GETGROUPS_T);
+  /* XXX - if (ngroups / sizeof (GETGROUPS_T) != size) out-of-range */
+  groups = scm_malloc (size);
+  for(i = ngroups - 1; i >= 0; i--)
+    {
+      SCM gid = SCM_VECTOR_REF (group_vec, i);
+      SCM_VALIDATE_INUM (0, gid);
+      groups [i] = SCM_INUM (gid);
+    }
+
+  result = setgroups (ngroups, groups);
+  free (groups);
+  if (result < 0)
+    SCM_SYSERROR;
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+#endif
+
 #ifdef HAVE_GETPWENT
 SCM_DEFINE (scm_getpwuid, "getpw", 0, 1, 0,
             (SCM user),


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

Grr.  I was comparing an unsigned value >= 0.  Ok, *this* should be
right.

Index: configure.in
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/configure.in,v
retrieving revision 1.213
diff -u -r1.213 configure.in
--- configure.in	7 Apr 2003 17:31:02 -0000	1.213
+++ configure.in	18 Apr 2003 17:48:43 -0000
@@ -583,7 +583,7 @@
     [Define if the system supports Unix-domain (file-domain) sockets.])
 fi
 
-AC_CHECK_FUNCS(socketpair getgroups setpwent pause tzset)
+AC_CHECK_FUNCS(socketpair getgroups setgroups setpwent pause tzset)
 
 AC_CHECK_FUNCS(sethostent   gethostent   endhostent   dnl
                setnetent    getnetent    endnetent    dnl
Index: libguile/posix.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/posix.c,v
retrieving revision 1.112
diff -u -r1.112 posix.c
--- libguile/posix.c	5 Apr 2003 19:10:22 -0000	1.112
+++ libguile/posix.c	18 Apr 2003 17:48:44 -0000
@@ -228,6 +228,41 @@
 #undef FUNC_NAME  
 #endif
 
+#ifdef HAVE_SETGROUPS
+SCM_DEFINE (scm_setgroups, "setgroups", 1, 0, 0,
+            (SCM group_vec),
+	    "Set the supplementary group IDs to those found in the vector\n"
+            "argument.")
+#define FUNC_NAME s_scm_setgroups
+{
+  size_t ngroups;
+  size_t size;
+  size_t i;
+  int result;
+  GETGROUPS_T *groups;
+
+  SCM_VALIDATE_VECTOR (0, group_vec);
+
+  ngroups = SCM_VECTOR_LENGTH (group_vec);
+  size = ngroups * sizeof (GETGROUPS_T);
+  /* XXX - if (ngroups / sizeof (GETGROUPS_T) != size) out-of-range */
+  groups = scm_malloc (size);
+  for(i = 0; i < ngroups; i++)
+    {
+      SCM gid = SCM_VECTOR_REF (group_vec, i);
+      SCM_VALIDATE_INUM (0, gid);
+      groups [i] = SCM_INUM (gid);
+    }
+
+  result = setgroups (ngroups, groups);
+  free (groups);
+  if (result < 0)
+    SCM_SYSERROR;
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+#endif
+
 #ifdef HAVE_GETPWENT
 SCM_DEFINE (scm_getpwuid, "getpw", 0, 1, 0,
             (SCM user),


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: setgroups

[Paul Jarc]

I wrote:
> +  result = setgroups (ngroups, groups);
> +  free (groups);
> +  if (result < 0)
> +    SCM_SYSERROR;

Should errno be saved and restored here to avoid interference from
free()?  The C standard says: "The value of errno may be set to
nonzero by a library function call whether or not there is an error,
provided the use of errno is not documented in the description of the
function in this International Standard.", and the description of
free() doesn't mention errno.  But if we're already ignoring this
issue in the rest of the code, I won't bother.  (Or I'll work on a
patch later to address as many instances of the problem as I can
find.)


paul


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel