Bug in the deluser applet

[tito]

On Wednesday 18 February 2015 09:14:42 you wrote:
> On Wed, Feb 18, 2015 at 8:09 AM, Laszlo Papp <lpapp at kde.org> wrote:
> > On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp <lpapp at kde.org> wrote:
> >> I thought about this, too, when writing the patch, but I rejected it because
> >> it is not simpler, in fact more complex, it is also not the right layer for
> >> the change. In addition, it would also be somewhat slower to execute.
> >
> > Just to elaborate a bit more on this as I was sending that on my phone:
> >
> > 1) Why do you want to make a few lines more simpler to spare a few
> > lines to introduce bad implementation and slower operation?
> >
> > 2) Also, the update_passwd function already has all the structure to
> > handle this through the file, name and member variables. It looks like
> > the natural place to utilize the existing infrastructure.
> >
> > 3) Basically, you would go through the groups once in the embedded
> > list and then you would go through again rather than just doing the
> > thing correct in the first place at the first iteration. So basically,
> > you would have double embedded list iteration. Even if the group list
> > was stored in memory for _each_ user, it would still slightly be
> > slower and I would argue that wasting memory for potentially a big
> > file could defeat busybox's purpose in the first place. Anyway,
> > busybox's design goal should be to be as fast as possible. Personal
> > preferences should neither slow it down, nor make it use more memory
> > than needed to achieve the task.
> 
> I would actually even go further than that, namely: the same-named
> group deletion should probably be integrated into my loop so that one
> embedded iteration could deal with the group file changes rather than
> two separate.
> 

Hi,

you can try to write a patch, but i suspect that it could be difficult
with the current update_password interface without breaking the
deluser from group use case.
If you opt for doing the changes in deluser instead some useful 
get_groups code is in the id applet and could be moved to libbb as
function. This will of course be slower as more iterations are done
through the passwd/group files.

Ciao,
Tito 

> >> On Wednesday, February 18, 2015, tito <farmatito at tiscali.it> wrote:
> >>> On Tuesday 17 February 2015 17:52:06 Laszlo Papp wrote:
> >>>
> >>>> Hi,
> >>>
> >>>>
> >>>
> >>>> 1) busybox addgroup user
> >>>
> >>>> 2) busybox adduser -G user testuser1
> >>>
> >>>> 3) busybox adduser -G user testuser2
> >>>
> >>>> 4) busybox deluser testuser1
> >>>
> >>>> 5) grep user /etc/passwd /etc/group
> >>>
> >>>>
> >>>
> >>>> ...
> >>>
> >>>> /etc/passwd:testuser2:x:1010:1007:Linux User,,,:/home/testuser2:/bin/bash
> >>>
> >>>> ...
> >>>
> >>>> /etc/group:user:x:1007:testuser,testuser2
> >>>
> >>>> ^^^^^^
> >>>
> >>>> ...
> >>>
> >>>>
> >>>
> >>>> This is not how the desktop util behaves and I agree with the desktop
> >>>
> >>>> util in this case. A non-existent user should not remain in
> >>>
> >>>> /etc/group.
> >>>
> >>>>
> >>>
> >>>> Do you agree or disagree?
> >>>
> >>>>
> >>>
> >>>> Cheers, L.
> >>>
> >>>
> >>>
> >>> Hi,
> >>>
> >>> i agree that this case is not covered by actual bb's deluser code.
> >>>
> >>> What we cover at the moment is:
> >>>
> >>>
> >>>
> >>> deluser USER (and group with same name if empty)
> >>>
> >>> deluser USER from GROUP
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> The desktop util behaviour could be added tough.
> >>>
> >>> By looking at the pseudo code you posted in a later
> >>>
> >>> mail I wonder if it could be easier to change
> >>>
> >>> deluser rather than update_passwd which is more complex.
> >>>
> >>> Like:
> >>>
> >>>
> >>>
> >>> do_deluser
> >>>
> >>> get_user_group_list
> >>>
> >>> do_delgroup of primary group
> >>>
> >>> while_list
> >>>
> >>> do_deluser_from_group
> >>>
> >>> or
> >>>
> >>> do_deluser
> >>>
> >>> while getgrent
> >>>
> >>> do_deluser_from_group(user, grname)
> >>>
> >>> do_delgroup of primary group
> >>>
> >>>
> >>>
> >>> Ciao,
> >>>
> >>> Tito
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20150218/9dbef49e/attachment-0001.html>