coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coreutils] A more forgiving rm.


From: Dan Hipschman
Subject: Re: [coreutils] A more forgiving rm.
Date: Tue, 24 Aug 2010 16:52:13 -0700
User-agent: Mutt/1.5.18 (2008-05-17)

On Tue, Aug 24, 2010 at 12:03:17PM -0600, Eric Blake wrote:
> Right now, I'm roughly 70-30 against adding a black-listing option,  
> although you may want to get feedback from more than just me.

Thanks for the summary.  These numbers actually make me feel like I
know where I stand a little better. :-)

> If we do add it, I don't like the name --warnings.  I'd rather name it  
> something like --black-list=FILE, and make the user explicitly pass in  
> the file name containing the blacklist, rather than assuming a default  
> location (under the assumption that anyone using this on a regular basis  
> will set up a shell alias or wrapper script to supply their preferred  
> location, be it ~/.rmfd/warn.list or somewhere else, rather than always  
> typing the option name).  We don't have any other precedence of a  
> hard-coded configuration locations in coreutils, and we DO have  
> precedence of dircolors taking an arbitrary location for determining a  
> user's preferred ls coloring, which has in turn been easily modified by  
> distros into shell startup scripts which can then supply a  
> distro-specific file location such as ~/.dircolors, all without having  
> to hard-code that into coreutils itself.

I have no problem doing that.

> Also, I would rather not burning a short-option letter -w until we know  
> whether such a feature will be widely-used enough to warrant that.

That is also not a problem to change.

> Doing a two pass traversal (once to see if anything blacklisted might be  
> encountered, and again when doing the actual deletion) is inherently  
> racy; I can see your intent of making the entire operation abortable if  
> any one black-listed file is among a pile of files rather than getting  
> half-way through the operation before encountering the first problem  
> file, but that is entirely new behavior compared to the standardized  
> approach of a single pass.  There is no way to avoid races with such a  
> new option, and I fear that the corner cases caused by such races may  
> cause more surprises than what black-listing intends to solve on the  
> surface.

Here is where I would start to argue. ;-) The point is not to prevent
removal of files (if so I would not prompt, I would just skip them or
fail as soon as one was found).  The point is to catch mistakes.  My
thinking here is that if a file that the user put in the list is going
to be removed, they are probably either mistaken about what directory
they are in, accidentally pasted the wrong thing (perhaps with an
embedded newline to trigger the execution), made a typo, or in some
other way botched their assumptions.  At that point, they probably
don't want to run the command at all, so the initial check is very
important.

If anything I would rather remove the second-pass checking.  Then
would this fall into the separate program category (run it as a front
end to rm)?  It seems awkward to do that, though, as it would have to
understand many of rm's options and it doesn't seem useful to any
other programs.

> Rather than requiring every line be an absolute file name (inherently  
> broken, given that filenames can contain newlines), I'd rather see  
> something that lets you intersperse comments, as well as use a  
> combination of globs and escape sequences for literal representation of  
> *, \n, #, etc.  In fact, you could even use extended globs such as  
> '/dir/**' to black-list both the directory and all its contents (which  
> would then obviate your hack in point 3 designed to protect someone from  
> doing 'rm dir/*' without first explicitly blacklisting all of dir's  
> contents).  Additionally, I don't see why you require absolute pathnames  
> (what if I want to black-list the relative name 'Makefile', regardless  
> of the directory I am in?).  But all of this will result in yet more  
> complexity for a new option.

Comments seem a bit excessive when all you've got is a list of paths,
but if it is going to get more complicated, those could easily be
added.  To be robust, yes, we need escape characters.  Since I was
only testing the waters on my own system, I hadn't thought about that.

I decided to go with absolute paths because, again, this isn't
intended to be an option like --exclude.  It's meant to catch
mistakes.  For that relative paths and globs didn't seem as useful.

> Finally, this only helps if you actually use 'rm' and the new option.  
> But it doesn't blacklist anything from any other means of deletion, such  
> as a script that calls 'rm', or manual use of 'mv', or even deletion  
> done by gui management tools.  In other words, it is a false sense of  
> security, and too easily bypassed.  I think there are much more secure  
> ways of protecting important files, such as ACLs, SELinux labeling, and  
> immutable attributes, which, by virtue of being kernel-enforced, will  
> apply to ALL programs and not just rm.  So why bloat rm with a  
> half-baked feature, when a full-baked feature is already available from  
> the kernel?

It seems like I keep coming up against this argument so please help me
find a better way to describe the goal of these changes.  The purpose
is not to try to protect these files from anything except human
stupidity.  The fd in rmfd is "for dummies".  Please think of this as
a super -I option.

>> 4. Add color to the warning prompt when stderr is a tty.
>
> This sounds like an independent change.  In general, adding the option  
> to colorize the prompt and/or verbose output of rm, mv, cp, and so on,  
> should be proposed as an independent patch series, rather than tying it  
> to an (as-yet) controversial new option.

That's fine.

> I've checked the FSF copyright records; while you still have assignment  
> on file, your employers' disclaimer appears to have expired as of June  
> 2008.  So as not to taint myself until this issue is resolved, I have  
> not looked at your patches for now.

That's not a problem since I don't work for that company anymore.  I'd
be happy to re-sign papers if need be.

> Now, if the rest of my email has been a bit discouraging, let me end on  
> a more positive note.  Thanks for taking the time to contribute, even if  
> in the end your patches are not used upstream.  Because this is open  
> source, you are more than welcome to run your custom rm in your  
> environment, and get the benefits that you see in your patch, even if  
> you have not managed to convince others of the same.  I wish more people  
> would approach feature requests with patches rather than proposals.

Thanks!  Hopefully other people will find it useful.  On top of that, I
feel like I've learned enough about the FTS family of functions that if I
have some spare time I could take on the todo task for copy.c. :-)

Thanks for the feedback.
Dan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]