bug#39364: [PATCH] rmdir: fix clobbered errno

[Pádraig Brady]

On 31/01/2020 17:51, Pádraig Brady wrote:
> On 31/01/2020 01:46, Matthew Pfeiffer wrote:
> > 'rmdir --ignore-fail-on-non-empty' would not report errors on non-empty
> > directories that fail for a different reason.
> > ---
> >    src/rmdir.c | 10 ++++++----
> >    1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/rmdir.c b/src/rmdir.c
> > index c9f417957..7b253ab0d 100644
> > --- a/src/rmdir.c
> > +++ b/src/rmdir.c
> > @@ -133,18 +133,19 @@ remove_parents (char *dir)
> >            prog_fprintf (stdout, _("removing directory, %s"), quoteaf (dir));
> >    
> >          ok = (rmdir (dir) == 0);
> > +      int rmdir_errno = errno;
> >    
> >          if (!ok)
> >            {
> >              /* Stop quietly if --ignore-fail-on-non-empty. */
> > -          if (ignorable_failure (errno, dir))
> > +          if (ignorable_failure (rmdir_errno, dir))
> >                {
> >                  ok = true;
> >                }
> >              else
> >                {
> >                  /* Barring race conditions, DIR is expected to be a directory. 
> >  */
> > -              error (0, errno, _("failed to remove directory %s"),
> > +              error (0, rmdir_errno, _("failed to remove directory %s"),
> >                         quoteaf (dir));
> >                }
> >              break;
> > @@ -233,12 +234,13 @@ main (int argc, char **argv)
> >    
> >          if (rmdir (dir) != 0)
> >            {
> > -          if (ignorable_failure (errno, dir))
> > +          int rmdir_errno = errno;
> > +          if (ignorable_failure (rmdir_errno, dir))
> >                continue;
> >    
> >              /* Here, the diagnostic is less precise, since we have no idea
> >                 whether DIR is a directory.  */
> > -          error (0, errno, _("failed to remove %s"), quoteaf (dir));
> > +          error (0, rmdir_errno, _("failed to remove %s"), quoteaf (dir));
> >              ok = false;
> >            }
> >          else if (remove_empty_parents)
>
> This looks like a regression introduced in v6.10-21-ged5c4e7

For reference, this was originally discussed at:
https://lists.gnu.org/archive/html/bug-coreutils/2008-01/msg00283.html

> I.E. is_empty_dir() is globbering errno, and thus
> a non specific and non terminating warning is output,
> rather than a specific error, and exit.

Actually I think the key issue is not errno handling,
but a logic error fixed with:

@@ -102,7 +102,7 @@ ignorable_failure (int error_number, char const *dir)
   return (ignore_fail_on_non_empty
           && (errno_rmdir_non_empty (error_number)
               || (errno_may_be_empty (error_number)
-                  && is_empty_dir (AT_FDCWD, dir))));
+                  && ! is_empty_dir (AT_FDCWD, dir))));


Attached is a full patch to address these issues.
cheers,
Pádraig

rmdir-ignore-perm.patch
Description: Text Data