Re: [Toybox] [CLEANUP] groupdel

2014-07-07 Thread Ashwini Sharma
On Mon, Jul 7, 2014 at 10:25 AM, Rob Landley r...@landley.net wrote:

 Haven't checked this in yet because it needs more testing, but patch is
 attached:

 GROUP is always toys.optargs[toys.optc-1] so let's move the getgrnam()
 before the two if/else cases so it's not duplicated. (Speaking of which,
 in the original code getgrnam() with the corresponding error message
 gets called twice in a row?)

 First question: are we ok hardwiring in the colon-separated /etc/groups
 file? (Because I thought android had some magic database instead?) I'm
 going to assume that's ok for now...

 I got confused by the man page for getgrnam pointing me to man 5 group,
 the second of which said that gr_mem is a comma separated list. But in
 the first it's a char *array[] which I _assume_ has a null terminator at
 the end, but it never SAYS so. (That's what the man page in Ubuntu 12.04


The second one explain about the format of the /etc/group file, where
gr_mem si a comma
separated list. The first one is the structure representation of the same,
as a *array[].
The null termination is not mentioned but is observed to be there.

says, anyway. Maybe it's stale?) That tangent got me to write a
 comma_find() function that should at least be useful elsewhere.


 The found == -1 part is using xprintf() instead of error_exit() because
 it wants to exit with status 0? And to print to stdout instead of
 stderr? And while we're at it, removing a user from a group that has no
 users in it doesn't produce this message either?

 Why do we call getpwnam() at all? Just to confirm it's an actual user?
 (Not being there is an error, can it be there and _not_ be a valid user?)


when  a user is added to the group, it is validated for its existence,  On
deletion
of a user from system it is removed from all the groups it belongs to, may
be due
to this __it__ not being there in case of removal from group is treated as
error.


 Ooh, _subtle_: due to xexec() not necessarily doing an actual exec()
 each time, we should call setpwent() before our first getpwent() because
 it's yet more process state that doesn't necessarily get reset between
 calls. (Possibly endpwent() should be part of the toy_init() stuff?) Or
 maybe endpwent() is better?

 ___
 Toybox mailing list
 Toybox@lists.landley.net
 http://lists.landley.net/listinfo.cgi/toybox-landley.net


___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net


[Toybox] [CLEANUP] groupdel

2014-07-06 Thread Rob Landley
Haven't checked this in yet because it needs more testing, but patch is
attached:

GROUP is always toys.optargs[toys.optc-1] so let's move the getgrnam()
before the two if/else cases so it's not duplicated. (Speaking of which,
in the original code getgrnam() with the corresponding error message
gets called twice in a row?)

First question: are we ok hardwiring in the colon-separated /etc/groups
file? (Because I thought android had some magic database instead?) I'm
going to assume that's ok for now...

I got confused by the man page for getgrnam pointing me to man 5 group,
the second of which said that gr_mem is a comma separated list. But in
the first it's a char *array[] which I _assume_ has a null terminator at
the end, but it never SAYS so. (That's what the man page in Ubuntu 12.04
says, anyway. Maybe it's stale?) That tangent got me to write a
comma_find() function that should at least be useful elsewhere.

The found == -1 part is using xprintf() instead of error_exit() because
it wants to exit with status 0? And to print to stdout instead of
stderr? And while we're at it, removing a user from a group that has no
users in it doesn't produce this message either?

Why do we call getpwnam() at all? Just to confirm it's an actual user?
(Not being there is an error, can it be there and _not_ be a valid user?)

Ooh, _subtle_: due to xexec() not necessarily doing an actual exec()
each time, we should call setpwent() before our first getpwent() because
it's yet more process state that doesn't necessarily get reset between
calls. (Possibly endpwent() should be part of the toy_init() stuff?) Or
maybe endpwent() is better?
diff -r ffc015bddb26 toys/pending/groupdel.c
--- a/toys/pending/groupdel.c	Sun Jul 06 23:43:29 2014 -0500
+++ b/toys/pending/groupdel.c	Sun Jul 06 23:53:13 2014 -0500
@@ -13,63 +13,70 @@
   default n
   help
 usage: delgroup [USER] GROUP
-groupdel GROUP
+usage: groupdel GROUP
 
-Delete a group or delete a user from a group
+Delete a group or remove a user from a group
 */
 
 #define FOR_groupdel
 #include toys.h
 
+char *comma_find(char *name, char *list)
+{
+  int len = strlen(name);
+
+  while (*list) {
+while (*list == ',') list++;
+if (!strncmp(name, list, len)  (!list[len] || list[len]==','))
+  return list;
+while (*list  *list!=',') list++;
+  }
+
+  return 0;
+}
+
 void groupdel_main(void)
 {
-  struct group *grp = NULL;
-  char *entry = NULL;
+  struct group *grp = getgrnam(toys.optargs[toys.optc-1]);
+  char *entry = 0;
 
-  if (toys.optc == 2) {  //del user from group
-//toys.optargs[0]- user, toys.optargs[1] - group
-if (!getpwnam(toys.optargs[0])) 
-  error_exit(user '%s' does not exist, toys.optargs[0]);
-if (!(grp = getgrnam(toys.optargs[1]))) 
-  error_exit(group '%s' does not exist, toys.optargs[1]);
-if (!(grp = getgrnam(toys.optargs[1]))) 
-  error_exit(group '%s' does not exist, toys.optargs[1]);
-if (!grp-gr_mem) return;
-else {
-  int i, found = -1;
+  if (!grp) perror_exit(group '%s', toys.optargs[toys.optc-1]);
 
-  for (i = 0; grp-gr_mem[i]  (found == -1); i++)
-if (!strcmp(grp-gr_mem[i], *toys.optargs)) found = i;
+  // delete user from group
+  if (toys.optc == 2) {
+int i, len = 0, found = -1;
+char *s;
 
-  if (found == -1) {
-xprintf(%s: The user '%s' is not a member of '%s'\n, toys.which-name,
-toys.optargs[0], toys.optargs[1]);
-return;
-  }
-  entry = xstrdup();
-  for (i=0; grp-gr_mem[i]; i++) {
-if (found != i) { //leave out user from grp member list
-  if (i  *entry) strcat(entry, ,);
-  entry = xrealloc(entry, strlen(entry) + strlen(grp-gr_mem[i]) + 2);
-  strcat(entry, grp-gr_mem[i]);
-}
+xgetpwnam(*toys.optargs);
+if (grp-gr_mem) {
+  for (i = 0; grp-gr_mem[i]; i++) {
+if (found == -1  !strcmp(*toys.optargs, grp-gr_mem[i])) found = i;
+else len += strlen(grp-gr_mem[i]) + 1;
   }
 }
-  } else {//delete the group
-struct passwd *pw = NULL;
+if (found == -1)
+  error_exit(user '%s' not in group '%s', *toys.optargs, toys.optargs[1]);
 
-if (!(grp = getgrnam(*toys.optargs))) 
-  error_exit(group '%s' doesn't exist, *toys.optargs);
-//is it a primary grp of user
-while ((pw = getpwent())) {
-  if (pw-pw_gid == grp-gr_gid) {
-endpwent();
-error_exit(can't remove primary group of user '%s', pw-pw_name);
-  }
+entry = s = xmalloc(len);
+for (i = 0; grp-gr_mem[i]; ) {
+  if (i) *(s++) = ',';
+  s = stpcpy(s, grp-gr_mem[i]);
 }
+
+  // delete group
+  } else {
+struct passwd *pw;
+
+endpwent(); // possibly this should be in toy_init()?
+for (;;) {
+  if (!(pw = getpwent())) break;
+  if (pw-pw_gid == grp-gr_gid) break;
+}
+if (pw) error_exit(can't remove primary group of user '%s', pw-pw_name);
 endpwent();
   }
+
   update_password(/etc/group,