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", grp->gr_name, entry); update_password("/etc/gshadow", grp->gr_name, entry); - free(entry); + if (CFG_TOYBOX_FREE) free(entry); }
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net