GNU bug report logs - #44889
[PATCH] rm: do not skip extra files when removal of empty directories fails

Previous Next

Package: coreutils;

Reported by: Nick Alcock <nick.alcock <at> oracle.com>

Date: Thu, 26 Nov 2020 15:17:03 UTC

Severity: normal

Tags: patch

Merged with 44883, 44884

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Pádraig Brady <P <at> draigBrady.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#44883: closed ("rm -rf *" performs an extra skip when it
 encounters an immutable empty directory)
Date: Thu, 26 Nov 2020 19:41:02 +0000
[Message part 1 (text/plain, inline)]
Your message dated Thu, 26 Nov 2020 19:39:54 +0000
with message-id <5b72061f-dc81-118f-bfa1-95d968e3900d <at> draigBrady.com>
and subject line Re: bug#44889: [PATCH] rm: do not skip extra files when removal of empty directories fails
has caused the debbugs.gnu.org bug report #44889,
regarding "rm -rf *" performs an extra skip when it encounters an immutable empty directory
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
44889: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=44889
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Nishant Nayan <nishant.nayan <at> oracle.com>
To: bug-coreutils <at> gnu.org
Subject: "rm -rf *" performs an extra skip when it encounters an immutable
 empty directory
Date: Thu, 26 Nov 2020 16:01:15 +0530
Hi,

The 'rm' utility' is skipping a mutable file when it encounters an 
immutable empty
directory (while deleting group of files and directories). 
Version(/8.22-18.0.1)/

Description: The bug is that rm skips an extra file while it encounters
an immutable empty directory. For example, on doing an "rm -rf *" on "a 
b c foo x y z",
where a,b,c,x,y,z are mutable files and foo is an immutable empty 
directory, the output
was "foo x", as soon as rm encounters an immutable directory, it skips 
it's immediate
next file(x in this case) and deletes all other files.

I would like to know if it's a known bug?


Regards
Nishant Nayan



[Message part 3 (message/rfc822, inline)]
From: Pádraig Brady <P <at> draigBrady.com>
To: Nick Alcock <nick.alcock <at> oracle.com>
Cc: elena.zannoni <at> oracle.com, nishant.nayan <at> oracle.com, bert.barbe <at> oracle.com,
 44889-done <at> debbugs.gnu.org, jemarch <at> gnu.org
Subject: Re: bug#44889: [PATCH] rm: do not skip extra files when removal of
 empty directories fails
Date: Thu, 26 Nov 2020 19:39:54 +0000
On 26/11/2020 18:49, Nick Alcock wrote:
> On 26 Nov 2020, Pádraig Brady verbalised:
> 
>> On 26/11/2020 14:35, Nick Alcock wrote:
>>> diff --git a/src/remove.c b/src/remove.c
>>> index 2d40c55cd..1150cf179 100644
>>> --- a/src/remove.c
>>> +++ b/src/remove.c
>>> @@ -508,8 +508,7 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
>>>                s = excise (fts, ent, x, true);
>>>                fts_skip_tree (fts, ent);
>>>              }
>>> -
>>> -        if (s != RM_OK)
>>> +        else if (s != RM_OK)
>>>              {
>>>                mark_ancestor_dirs (ent);
>>>                fts_skip_tree (fts, ent);
>>
>> I think we'd still like to mark ancestors when failing to remove,
>> so that we don't prompt unnecessarily.
> 
> If this doesn't make the test fail, I'm fine with it.

Cool, the test still passes :)

>> I think it would be better to do:
>>
>> diff --git a/src/remove.c b/src/remove.c
>> index 2d40c55cd..adf948935 100644
>> --- a/src/remove.c
>> +++ b/src/remove.c
>> @@ -506,7 +506,8 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x)
>>               /* When we know (from prompt when in interactive mode)
>>                  that this is an empty directory, don't prompt twice.  */
>>               s = excise (fts, ent, x, true);
>> -            fts_skip_tree (fts, ent);
>> +            if (s == RM_OK)
>> +              fts_skip_tree (fts, ent);
>>             }
> 
> I don't really understand what mark_ancestor_dirs is doing, but as long
> as it's not making the file disappear and the new test still passes I'm
> fine with this (though honestly the s= stuff is incredibly confusing and
> really should be using two distinct variables for result-of-prompt and
> result-of-excision to make it obvious what the flaming dingbats is going
> on).
> 
> Another worry of mine is that I don't understand why fts_skip_tree is
> skipping an entry *other* than ent the second time it's called. Naively
> I'd have assumed fts_skip_tree (fts, ent) would be idempotent: calling
> it repeatedly with the same ent should do the same as calling it only
> once. But clearly this is not the case, so I'm misreading the code in
> gnulib and/or glibc somehow.

There is an fts_read() in fts_skip_tree() that's consuming the entry.

Pushed at:
https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=6bf108358

cheers,
Pádraig


This bug report was last modified 4 years and 176 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.