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

[Denys Vlasenko]

On Sunday 07 November 2010 15:38, Tito wrote:
> > >> What "standard" delgroup foo will do? I suspect it will
> > >> complain that haldaemon user's primary GID is 490
> > >> and therefore group foo can't be deleted.
> > >
> > > adduser prova
> > > Adding user `prova' ...
> > > Adding new group `prova' (1006) ...
> > > Adding new user `prova' (1004) with group `prova' ...
> > > adduser prova2 --ingroup prova
> > > Adding user `prova2' ...
> > > Adding new user `prova2' (1005) with group `prova' ...
> > 
> > And what grep prova /etc/passwd /etc/group shows
> > after these?

I need an answer to the above question.








> > > Test case 1: Removing user prova:
> > >
> > > deluser prova
> > 
> > Awww, my brainzzzz... Why do you delete *a user*?
> > We are trying to determine what is the correct behavior
> > of *delgroup*, right?
> 
> Yes, we do but we have to take into account 
> all corner cases.
> 
> > > Removing user `prova' ...
> > > Warning: group `prova' has no more members.
> > 
> > Seems like an erroneous message. Group prova
> > should still have at least prova2 user.
> 
> No, it is referring to members of group prova 
> like prova:1006:pippo,pluto
> primary groups will not show up here
> but only in /etc/passwd as
> user:uid:GID:...... 

I understand, but whoever reads this command may think that
there also are no users with the GID of this group.

IOW: message is lying.


> > Looks like code only checks /etc/group line
> > whether it contains additional usernames:
> > prova:x:1006:<any names here?>
> 
> No. It walks /etc/passwd to see
> if there is any user with GID=1006
> as it is possible to delete
> a group even if it has members
> (like prova:1006:pippo,pluto)
> and real delgroup will not complain
> (see later)

In this case it should find prova2 and therefore NOT emit that message.
It does emit the message, though.
Therefore I think your theory is not correct.


> > but not /etc/passwd for users with GID=1006.
> > I'd say this is a bug.
> > 
> > > userdel: Cannot remove group prova which is a primary group for another user.
> > 
> > Right, this means that prova2 line looks like this:
> > prova2:x:1005:1006:...........
> 
> Yes


> > > Done.
> > > grep prova /etc/group
> > > prova:x:1006:
> 
> Infact group prova was not deleted as referenced
> in /etc/passwd by user prova2 (gid=1006)
> > >
> > > Test case 2: Removing user prova2:
> > >
> > > deluser prova2
> > > Removing user `prova2' ...
> > > Warning: group `prova' has no more members.
> > > Done.
> > 
> > Attention. See _which_ group it deleted here?
> 
> None. as there is no group with name prova2.

What code is emitting "Warning: group `prova' has no more members" then?
Obviously, a code which tries to delete group "prova" - why would
code check it otherwise? HOW the jump from "user prova2"
to "group prova" occurred? Their names don't match!

> > It did not try to delete a group with the same name as user,
> > it looked at GID (2006), found which group it is (prova),
> > and then deleted it.
> 
> No, it just warned that group is empty as it always does (in the sense of no members).

But why the warning is about prova, not prova2?


> I'm aware now that the name matters
> only when trying to delgroup an UPG
> when doing deluser, in this case
> first check if there is a group with same name

I dont believe it looks for a group with the same name.
I think it looks for a group with the GID found in
deleted user's passwd line.

> as the user and if there is one
> check if it is used only by the user
> (walking /etc/passwd)


Can you determine which package your "standard" deluser/group tool
belongs? I would like to take a look at their source.


> So far what I'm sure about is:
> 
> 1) delgroup can delete groups with members without erroring out (agroup:100:member1,member2)

Sure, why not

> 2) delgroup checks if gid of group is referenced in /etc/passwd   as primary group of an user and errors out if gid is found

Yes.

> 3) deluser tries to delgroup the UPG:
> 	a) group with same name as user is found: do sanity checks and then remove
> 	b) group with same name as user is not found: do nothing and exit succes.

No (I think).


> Attached you find test version of deluser.c that should now do it the right
> way even if not displaying the same messages as the real version.
> I've tested it a little but it needs more cleanup and size reduction.
> Please take a look at it and help me to find the obvious bugs
> I'm not able to see right now.

I am applying this part:

                        if (!member) {
                                struct passwd *pw;
                                struct passwd pwent;
                                /* "delgroup GROUP" */
                                /* Check if the group is in use */
#define passwd_buf bb_common_bufsiz1
                                while (!getpwent_r(&pwent, passwd_buf, sizeof(passwd_buf), &pw)) {
                                        if (pwent.pw_gid == gid)
                                                bb_error_msg_and_die("'%s' still has '%s' as their primary group!", pwent.pw_name, name);
                                }
                                //endpwent();
                        }


Let's continue discussion on what "deluser foo" does.
-- 
vda