[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: Deluser: deleting the home folder
From: Laszlo Papp <lpapp () kde ! org>
Date: 2014-10-31 18:38:46
Message-ID: CAOMwXhNUGEkEdsqdLb85ncLRU620nB-vLwao_rsXaW=1-mJkgw () mail ! gmail ! com
[Download RAW message or body]
Guys, what is going on ??
On Mon, Aug 4, 2014 at 6:27 PM, Laszlo Papp <lpapp@kde.org> wrote:
> Indeed, thanks, and sorry. ^^
>
> On Mon, Aug 4, 2014 at 6:21 PM, Denys Dmytriyenko <denis@denix.org> wrote:
>>
>> Laszlo,
>>
>> I think you meant the other Denys, the one with RedHat that maintains
>> busybox.
>> I'm with Texas Instruments and maintain meta-ti/Arago in OE/Yocto world :)
>>
>> --
>> Denys
>>
>>
>> On Mon, Aug 04, 2014 at 06:10:40PM +0100, Laszlo Papp wrote:
>> > Denys, could you please provide feedback here?
>> >
>> > 1) Do you appreciate the feature?
>> >
>> > 2) If yes, which option would you like to see for it?
>> >
>> >
>> > On Wed, Jul 16, 2014 at 1:28 PM, Laszlo Papp <lpapp@kde.org> wrote:
>> >
>> > > 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 <lpapp@kde.org> wrote:
>> > >
>> > >>
>> > >>
>> > >>
>> > >> On Thu, Jul 3, 2014 at 10:28 PM, tito <farmatito@tiscali.it> wrote:
>> > >>
>> > >>> On Thursday 03 July 2014 22:38:23 you wrote:
>> > >>> > On Thu, Jul 3, 2014 at 8:31 PM, tito <farmatito@tiscali.it> wrote:
>> > >>> >
>> > >>> > > On Thursday 03 July 2014 14:51:11 you wrote:
>> > >>> > > > On Thu, Jul 3, 2014 at 12:59 PM, tito <farmatito@tiscali.it>
>> > >>> wrote:
>> > >>> > > >
>> > >>> > > > > On Thursday 03 July 2014 13:03:46 Laszlo Papp wrote:
>> > >>> > > > > > On Thu, Jul 3, 2014 at 11:10 AM, Laszlo Papp
>> > >>> > > > > > <lpapp@kde.org>
>> > >>> wrote:
>> > >>> > > > > >
>> > >>> > > > > > > commit 761fd153e340a14abccc0af89f2f6617faf2077f
>> > >>> > > > > > > Author: Laszlo Papp <lpapp@kde.org>
>> > >>> > > > > > > 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
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic