Re: I: [PATCH] coreutils-5.92 make_dir_parents regression

[Jim Meyering]

"Dmitry V. Levin" <address@hidden> wrote:
> "mkdir -p" and "install -d" now fail when creating final component
> of the file name when it already exists.
> Proposed patch with testcase is attached.
>
> 2005-10-24  Dmitry V. Levin  <address@hidden>
>
>       * lib/mkdir-p.c (make_dir_parents): When creating final component
>       of the file name, do not fail when it already exists.
>       * tests/install/d-slashdot: New test, for "install -d" failure.
>       * tests/install/Makefile.am (TESTS): Add d-slashdot.
>       * tests/mkdir/p-slashdot: New test, for "mkdir -p" failure.
>       * tests/mkdir/Makefile.am (TESTS): Add p-slashdot.

Thanks for the patch and test cases.
I'd already fixed it, but with code relying on an errno == EEXIST test.
So I've made yet another change (mostly your patch, with slightly
different scoping):

2005-10-24  Jim Meyering  <address@hidden>

        * mkdir-p.c (make_dir_parents): Like the code above it, don't
        rely on mkdir failing with a particular errno value (EEXIST).
        Based on a patch by Dmitry V. Levin.

Index: lib/mkdir-p.c
===================================================================
RCS file: /fetish/cu/lib/mkdir-p.c,v
retrieving revision 1.14
retrieving revision 1.15
diff -u -p -u -r1.14 -r1.15
--- lib/mkdir-p.c       24 Oct 2005 10:22:10 -0000      1.14
+++ lib/mkdir-p.c       24 Oct 2005 13:35:59 -0000      1.15
@@ -264,41 +264,25 @@ make_dir_parents (char const *arg,
         Create the final component of the file name.  */
       if (retval)
        {
-         bool just_created = (mkdir (basename_dir, mode) == 0);
-         if (just_created)
+         bool dir_known_to_exist = (mkdir (basename_dir, mode) == 0);
+         int mkdir_errno = errno;
+         struct stat sbuf;
+
+         if ( ! dir_known_to_exist)
+           dir_known_to_exist = (stat (basename_dir, &sbuf) == 0
+                                 && S_ISDIR (sbuf.st_mode));
+
+         if ( ! dir_known_to_exist)
            {
-             if (verbose_fmt_string)
-               error (0, 0, verbose_fmt_string, quote (dir));
-             fixup_permissions_dir = basename_dir;
+             error (0, mkdir_errno,
+                    _("cannot create directory %s"), quote (dir));
+             retval = false;
            }
          else
            {
-             if (errno != EEXIST)
-               {
-                 error (0, errno, _("cannot create directory %s"), quote 
(dir));
-                 retval = false;
-               }
-             else
-               {
-                 /* basename_dir exists.
-                    This is highly unlikely, but not impossible in a race.
-                    You can exercise this code by running a very slow
-                    mkdir -p a/nonexistent/c process and e.g., running
-                    touch a/nonexistent/c after a/nonexistent is created
-                    but before mkdir attempts to create `c'.
-
-                    If it's a directory, we're done.
-                    Otherwise, we must fail.  */
-                 struct stat sbuf;
-                 /* The stat may fail for a dangling link.  */
-                 if (stat (basename_dir, &sbuf) != 0
-                     || ! S_ISDIR (sbuf.st_mode))
-                   {
-                     error (0, 0, _("%s exists but is not a directory"),
-                            quote (basename_dir));
-                     retval = false;
-                   }
-               }
+             if (verbose_fmt_string)
+               error (0, 0, verbose_fmt_string, quote (dir));
+             fixup_permissions_dir = basename_dir;
            }
        }
     }