[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#12260: [patch] rm -d in coreutils 8.19
From: |
Jim Meyering |
Subject: |
bug#12260: [patch] rm -d in coreutils 8.19 |
Date: |
Thu, 23 Aug 2012 19:38:21 +0200 |
Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 08/23/2012 12:17 PM, Robert Day wrote:
>>> On 22 August 2012 23:23, Robert Day <address@hidden> wrote:
>>>
>>>> I've attached a patch which fixes this bug and adds a test of that code
>>>> path. The fixes can also be retrieved from
>>>> https://github.com/rkd91/coreutils_rm_di_patch.
>>>>
>>>
>>> I've made a couple of related fixes (comments/code niceness and adding a
>>> NEWS item), so I've attached a new patch and updated my github.
>>
>> Thanks for handling that Robert.
>> It's on the borderline for copyright assignment
>> (which I don't think you have?).
>> It's probably OK to waive in this case anyway.
>
> I agree that it's borderline, but also that it's ok.
>
>> I've not got time to fully squash/review your
>> patches just now, but we'll add them in soon.
>
> I'm going through it now, constructing a proper commit log, attributing
> the reporter, fixing NEWS to avoid the minor syntax-check failure,
> adjusting comment formatting, e.g.,
>
> diff --git a/src/remove.c b/src/remove.c
> index 06bc926..2dbb441 100644
> --- a/src/remove.c
> +++ b/src/remove.c
> @@ -199,7 +199,6 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
> else
> is_empty = false;
>
> -
> /* When nonzero, this indicates that we failed to remove a child entry,
> either because the user declined an interactive prompt, or due to
> some other failure, like permissions. */
> @@ -249,8 +248,8 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
>
> case DT_DIR:
> /* Unless we're either deleting directories or deleting
> - * recursively, we want to raise an EISDIR error rather than
> - * prompting the user */
> + recursively, we want to raise an EISDIR error rather than
> + prompting the user */
> if (!x->recursive
> && !(x->remove_empty_directories && is_empty))
> {
>
> I'll post for final review shortly.
Here's another few code changes:
The first hunk is because I don't like dangling single-line brace-less
"else" bodies when the then block has braces, and beside that first hunk
saves one line.
The second factors out an "!", and makes it a little easier to read.
The third removes a stray doubled space.
diff --git a/src/remove.c b/src/remove.c
index 2dbb441..69faae6 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -190,14 +190,12 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
int dirent_type = is_dir ? DT_DIR : DT_UNKNOWN;
int write_protected = 0;
- bool is_empty;
+ bool is_empty = false;
if (is_empty_p)
{
is_empty = is_empty_dir (fd_cwd, filename);
*is_empty_p = is_empty ? T_YES : T_NO;
}
- else
- is_empty = false;
/* When nonzero, this indicates that we failed to remove a child entry,
either because the user declined an interactive prompt, or due to
@@ -250,8 +248,7 @@ prompt (FTS const *fts, FTSENT const *ent, bool is_dir,
/* Unless we're either deleting directories or deleting
recursively, we want to raise an EISDIR error rather than
prompting the user */
- if (!x->recursive
- && !(x->remove_empty_directories && is_empty))
+ if ( ! (x->recursive || (x->remove_empty_directories && is_empty)))
{
write_protected = -1;
wp_errno = EISDIR;
@@ -425,7 +422,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
/* This is the first (pre-order) encounter with a directory
that we cannot delete.
Not recursive, and it's not an empty directory (if we're removing
- them) so arrange to skip contents. */
+ them) so arrange to skip contents. */
int err = x->remove_empty_directories ? ENOTEMPTY : EISDIR;
error (0, err, _("cannot remove %s"), quote (ent->fts_path));
mark_ancestor_dirs (ent);