On Monday 04 October 2010 08:10:11 Tito wrote:
> On Monday 04 October 2010 00:45:30 you wrote:
> > On Sunday 03 October 2010 21:18, Tito wrote:
> > > Hi,
> > > this patch partially reduces size of deluser/delgroup and adds better
> > > error checking and a few additional features:
> > >
> > > Pros:
> > > 1) size increase vs functionality is not bad. With all options enabled:
> > >
> > > scripts/bloat-o-meter busybox_old busybox_unstripped
> > > function old new delta
> > > deluser_main 189 214 +25
> > > .rodata 134994 135016 +22
> > > ------------------------------------------------------------------------------
> > > (add/remove: 0/0 grow/shrink: 2/0 up/down: 47/0) Total: 47
> > > bytes
> > >
> > > 2) proper error checking and reporting if removing non existent user/group
> > > 3) proper error checking and reporting if removing primary group of a user
> > > 4) deluser also removes user's primary group if ENABLE_DELGROUP is set,
> > > so no leftovers
> >
> > Why? Who said that group foo needs to be removed when user foo is being
> > removed?
> > Do other deluser utilities do this?
> Hi
> Yes at least on my Debian system if i create a user:
>
> adduser prova
> Adding user `prova' ...
> Adding new group `prova' (1006) ...
> Adding new user `prova' (1004) with group `prova' ...
> The home directory `/home/prova' already exists. Not copying from
> `/etc/skel'.
> Enter new UNIX password:
> Retype new UNIX password:
> passwd: password updated successfully
> Changing the user information for prova
> Enter the new value, or press ENTER for the default
> Full Name []:
> Room Number []:
> Work Phone []:
> Home Phone []:
> Other []:
> Is the information correct? [Y/n] debian:/home/tito/Desktop#
>
>
> grep prova /etc/pashadowetc/shadow /etc/group /etc/gshadow
> /etc/passwd:prova:x:1004:1006:,,,:/home/prova:/bin/bash
> /etc/shadow:prova:$6$I..Q9V.z$aoa7lBKlI68heRLDh9E0oMm8DUiBFO9MfBZgOYYjr/09v0Z1nPipjDPxz7TbbuwLmRslWZVdpI3Wr/G4IxDrl1:14886:0:99999:7:::
> /etc/group:prova:x:1006:
> /etc/gshadow:prova:!::
>
> deluser prova
> Removing user `prova' ...
> Warning: group `prova' has no more members.
> Done.
> grep prova /etc/passwd /etc/shadow /etc/group /etc/gshadow
> debian:/home/tito/Desktop#
>
>
> adduser command creates a group with the same name as the username,
> and makes this group the primary group for that user. This is called a user
> private group (UPG)
>
> So i test if a group with the same name exists and delete it,
> Maybe a check if the group is empty could be added but as
> we don't check for that at all in the current implementation
> and happily remove non-empty groups I've skipped that.
>
> >
> > > 5) maximum dead code optimization by the compiler with different options
> > > turned off
> > >
> > > Contra:
> > > 1) code is obfuscated, sorry can't do nothing about that it's my style.
> > > 2) Not known ....yet :-)
> > >
> > > The patch is tested and seems to work well for me.
> > > More testing by the list members is appreciated.
> > > Hints, critics and improvements by the list members are welcome.
> > > Please apply if you like it.
> >
> > + if (!member && getpwnam(name))
> > + bb_error_msg_and_die("is %s's primary
> > group", name);
> >
> > This looks unreadable. Can be rewritten to do the same,
> > but make more sense to human reader, as:
> >
> > if (!member) {
> > if (getpwnam(name) = 0) {
> > bb_error_msg_and_die("is %s's primary group", name);
> > }
> > }
> >
>
> Can change it if you like it that way, for my personal taste just 3 wasted
> lines :-)
>
> Should be:
>
> if (!member) {
> if (getpwnam(name)) {
> bb_error_msg_and_die("is %s's primary group", name);
> }
> }
>
> We check if getpwnam returns a struct passwd and in this case
> it is a primary group (group with the same name of a user).
>
> > The message also needs to be better. This:
> >
> > deluser: is foo's primary group: <strerror here>
> >
> > is incomprehensible. What are you trying to say?
> >
>
> Original error message on my system is:
>
> delgroup tito
> /usr/sbin/delgroup: `tito' still has `tito' as their primary group!
>
> I shortened it a little to:
>
> delgroup: is tito's primary group
>
> Could be changed. As i'm not a native english speaker I'm open
> to suggestions about a short and easy to understand error message
> for this use case.
>
>
> Ciao,
> Tito
>
Here v4,
just cosmetical changes for readability.
Ciao,
Tito
Signed-off-by: Tito Ragusa <[email protected]>
--- loginutils/deluser.c.orig 2010-09-22 22:05:25.000000000 +0200
+++ loginutils/deluser.c 2010-10-05 22:25:22.000000000 +0200
@@ -14,41 +14,50 @@
int deluser_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int deluser_main(int argc, char **argv)
{
- if (argc != 2
- && (!ENABLE_FEATURE_DEL_USER_FROM_GROUP
- || applet_name[3] != 'g'
- || argc != 3)
- ) {
- bb_show_usage();
- }
-
- if (geteuid())
- bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
+ char *name = argv[1];
+ char *member = NULL;
+ const char *pfile = bb_path_passwd_file;
+ const char *sfile = NULL;
+ bool do_deluser = (ENABLE_DELUSER && (!ENABLE_DELGROUP || applet_name[3] == 'u'));
- if (ENABLE_DELUSER && applet_name[3] == 'u') {
- /* deluser USER */
- if (update_passwd(bb_path_passwd_file, argv[1], NULL, NULL) < 0)
- return EXIT_FAILURE;
- if (ENABLE_FEATURE_SHADOWPASSWDS)
- if (update_passwd(bb_path_shadow_file, argv[1], NULL, NULL) < 0)
- return EXIT_FAILURE;
- } else if (ENABLE_DELGROUP) {
- /* delgroup ... */
- if (!ENABLE_FEATURE_DEL_USER_FROM_GROUP || argc != 3) {
- /* delgroup GROUP */
- if (update_passwd(bb_path_group_file, argv[1], NULL, NULL) < 0)
- return EXIT_FAILURE;
- if (ENABLE_FEATURE_SHADOWPASSWDS)
- if (update_passwd(bb_path_gshadow_file, argv[1], NULL, NULL) < 0)
+ switch (argc) {
+ case 3:
+ if (!ENABLE_FEATURE_DEL_USER_FROM_GROUP || do_deluser)
+ break;
+ name = argv[2];
+ member = argv[1];
+ /* Fallthrough */
+ case 2:
+ if (geteuid())
+ bb_error_msg_and_die(bb_msg_perm_denied_are_you_root);
+ if (do_deluser) {
+ xgetpwnam(name);
+ if (ENABLE_FEATURE_SHADOWPASSWDS)
+ sfile = bb_path_shadow_file;
+ } else {
+ do_delgroup:
+ xgetgrnam(name);
+ if (!member) {
+ if(getpwnam(name) != NULL) {
+ bb_error_msg_and_die("is %s's primary group", name);
+ }
+ }
+ pfile = bb_path_group_file;
+ if (ENABLE_FEATURE_SHADOWPASSWDS)
+ sfile = bb_path_gshadow_file;
+ }
+ do {
+ if (update_passwd(pfile, name, NULL, member) == -1)
return EXIT_FAILURE;
- } else {
- /* delgroup USER GROUP */
- if (update_passwd(bb_path_group_file, argv[2], NULL, argv[1]) < 0)
- return EXIT_FAILURE;
- if (ENABLE_FEATURE_SHADOWPASSWDS)
- if (update_passwd(bb_path_gshadow_file, argv[2], NULL, argv[1]) < 0)
- return EXIT_FAILURE;
- }
+ pfile = sfile;
+ sfile = NULL;
+ } while (pfile);
+
+ if (ENABLE_DELGROUP && do_deluser) {
+ do_deluser = 0;
+ goto do_delgroup;
+ }
+ return EXIT_SUCCESS;
}
- return EXIT_SUCCESS;
+ bb_show_usage();
}
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
