[Top][All Lists]

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

Re: [coreutils] A more forgiving rm.

From: Eric Blake
Subject: Re: [coreutils] A more forgiving rm.
Date: Tue, 24 Aug 2010 12:03:17 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20100806 Fedora/3.1.2-1.fc13 Mnenhy/0.8.3 Thunderbird/3.1.2

On 08/24/2010 11:10 AM, Dan Hipschman wrote:

I forked off my own copy of rm to play with and have added the
following functionality.

1. If the --warnings (-w) option is given, read the file
~/.rmfd/warn.list if it exists.  The format is one absolute path per
line.  Before any files are removed, see if any file in the list would
be removed by the invocation, and if so, warn and prompt the user for
each file found whether it is OK to continue.  Note that this
overrides --force.  If the user declines, exit with status
EXIT_FAILURE.  If the user allows it, continue with the usual
semantics.  Note that rm also checks as it is removing files, so if it
finds a file that is in the list that wasn't warned about during the
initial check, it will still prompt, but the "abort the entire
operation" semantics can't be carried out.

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.

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.

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.

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.

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.

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?

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.

I would appreciate feedback on the concept or the code.  I am still
interested in hearing if this could be considered as an extension to
GNU rm on the grounds that the extra code is not substantial
(Coreutils' rm stripped is 47K on my system and rmfd's rm is 50K) and
the performance hit and POSIX noncompliance is entirely opt-in.

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.

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.

Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library

reply via email to

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