This patch was sent to the mailing list two weeks ago without further
comment from the maintainer...


On Fri, Jul 4, 2014 at 11:15 AM, Laszlo Papp <[email protected]> wrote:

>
>
>
> On Thu, Jul 3, 2014 at 10:28 PM, tito <[email protected]> wrote:
>
>> On Thursday 03 July 2014 22:38:23 you wrote:
>> > On Thu, Jul 3, 2014 at 8:31 PM, tito <[email protected]> wrote:
>> >
>> > > On Thursday 03 July 2014 14:51:11 you wrote:
>> > > > On Thu, Jul 3, 2014 at 12:59 PM, tito <[email protected]> wrote:
>> > > >
>> > > > > On Thursday 03 July 2014 13:03:46 Laszlo Papp wrote:
>> > > > > > On Thu, Jul 3, 2014 at 11:10 AM, Laszlo Papp <[email protected]>
>> wrote:
>> > > > > >
>> > > > > > > commit 761fd153e340a14abccc0af89f2f6617faf2077f
>> > > > > > > Author: Laszlo Papp <[email protected]>
>> > > > > > > Date:   Thu Jul 3 11:06:58 2014 +0100
>> > > > > > >
>> > > > > > >     Add optional home directory removal support to deluser
>> > > > > > >
>> > > > > > > diff --git a/loginutils/deluser.c b/loginutils/deluser.c
>> > > > > > > index e39ac55..67b744b 100644
>> > > > > > > --- a/loginutils/deluser.c
>> > > > > > > +++ b/loginutils/deluser.c
>> > > > > > > @@ -11,9 +11,10 @@
>> > > > > > >   */
>> > > > > > >
>> > > > > > >  //usage:#define deluser_trivial_usage
>> > > > > > > -//usage:       "USER"
>> > > > > > > +//usage:       "[-h] USER"
>> > > > > > >  //usage:#define deluser_full_usage "\n\n"
>> > > > > > >  //usage:       "Delete USER from the system"
>> > > > > > > +//usage:       "\n    -h   Remove the home directory"
>> > > > > > >
>> > > > > > >  //usage:#define delgroup_trivial_usage
>> > > > > > >  //usage:       IF_FEATURE_DEL_USER_FROM_GROUP("[USER]
>> ")"GROUP"
>> > > > > > > @@ -35,11 +36,15 @@ int deluser_main(int argc, char **argv)
>> > > > > > >         /* Name of shadow or gshadow file */
>> > > > > > >         const char *sfile;
>> > > > > > >         /* Are we deluser or delgroup? */
>> > > > > > > +    struct passwd *pw = 0;
>> > > > > > >
>> > > > > >
>> > > > > > This could probably be one line below not to distract the
>> comment and
>> > > > > > corresponding variable declaration. Although, ideally, this
>> would
>> > > need to
>> > > > > > go to the "case 2" branch, but I did not want to introduce a new
>> > > block
>> > > > > > there with re-indenting many lines. Also, do you prefer "NULL"
>> > > instead of
>> > > > > > "0"?
>> > > > > >
>> > > > > > Let me know what the preferred style is...
>> > > > > >
>> > > > > > The patch is tested with and without "-h" and it works. The
>> option
>> > > > > > selection is "-h" which reminds some people the canonical
>> "help",
>> > > but on
>> > > > > > the contrary, this is also what is used for adduser to create
>> the
>> > > home
>> > > > > > directory, so I picked it up for being consistent. Again, let
>> me know
>> > > > > your
>> > > > > > preference ...
>> > > > > >
>> > > > >
>> > > > > Hi,
>> > > > > couldn't we change -h as it conflicts with -h/--help and use -r
>> as in
>> > > > > --remove-home:
>> > > > >
>> > > >
>> > > > Well, I prefer consistency, otherwise it will become to effectively
>> use
>> > > the
>> > > > applets. After all, if you do not type anything, you will get the
>> help
>> > > > output, or misuse it, so why would we bloat the applet code with
>> that?
>> > > >
>> > >
>> > > Hi,
>> > > where is the bloat in doing:
>> > >
>> > >  //usage:#define deluser_trivial_usage
>> > > -//usage:       "USER"
>> > > +//usage:       "[-r] USER"
>> > >  //usage:#define deluser_full_usage "\n\n"
>> > >  //usage:       "Delete USER from the system"
>> > > +//usage:       "\n    -r   Remove the home directory"
>> > >
>> >
>> > That does not make sense to me, I am afraid. It might be possible later
>> to
>> > remove other config data, too. It is probably not acceptable, thus it is
>> > not done so even on desktop.
>> >
>> >
>> > > and
>> > >
>> > >     int do_delhome = 0;
>> > >     if (getopt32(argv, "r") & 1) { ++argv; --argc; do_delhome = 1; }
>> > >
>> > > or maybe simply:
>> > >
>> > >     int do_delhome = getopt32(argv, "r"):
>> > >     argc -= optind;
>> > >     argv += optind;
>> > >
>> >
>> > This looks worse than a simple increment to me, but it is such a minor
>> > detail that I do not think it is too relevant.
>> >
>> > -h is nice and consistent. I do not know why you would want help option
>> two
>> > when it only has one option. You would double the option number. It
>> would
>> > be an overkill in this case.
>>
>> Hi,
>> I want not to double the number of options I just suggest to use
>> -r instead of -h because:
>>
>
> As already replied, -r is not clear an option. I was thinking about -h and
> --remove-home in the beginning. I think anything else is bad choice because
> it is inconsistent with the rest of the world. I prefer local consistency
> within busybox, this I picked up -h, but if Denys would like to avoid that
> local consistency, I suggest --remove-home to at least have some
> consistency, namely with the desktop.
>
>
>> 1) -h is mostly used for help (with a few exceptions I am aware of).
>> 2)  on the desktop:
>>       a) deluser uses  --remove-home   ( Remove the home directory of the
>> user and its mailspool)
>>       b) userdel uses  -r, --remove  (Files in the user's home directory
>> will be removed along with the
>>                                                       home directory
>> itself and the user's mail spool.)
>>          therefore using -r would be consistent and logic to use.
>>
>
> That is exactly why it would be inconsistent and not logical IMHO. "-r"
> means remove "everything" and definitely not just home.
>
>
>>
>> > By the way, I like the bikeshed pink. ;-)
>>
>>      This was just a hint to reduce codesize,
>>      untested  so I will not bet on it.
>>
>>      int do_delhome = getopt32(argv, "r"):
>>      argc -= optind;
>>      argv += optind;
>>
>
> I do not see any benefit of it for one option; it also seems to make the
> code longer IMHO.
>
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to