On Tuesday 05 October 2010 22:38:14 Tito wrote:
> 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
> 

Hi Denys,
is there still interest for this patch?
Should something be changed?
or could it be dropped?

Ciao,
Tito
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to