On Wednesday 18 February 2015 09:14:42 you wrote: > On Wed, Feb 18, 2015 at 8:09 AM, Laszlo Papp <[email protected]> wrote: > > On Wed, Feb 18, 2015 at 7:34 AM, Laszlo Papp <[email protected]> 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 <[email protected]> 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 >
_______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
