[PATCH] 2nd attempt at deluser/delgroup size reduction and improvements

[Tito]

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