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

Denys Vlasenko vda.linux at googlemail.com
Sun Oct 3 22:45:30 UTC 2010


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?


> 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);
		}
	}

The message also needs to be better. This:

deluser: is foo's primary group: <strerror here>

is incomprehensible. What are you trying to say?

-- 
vda


More information about the busybox mailing list