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. >> 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
