[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: I: [PATCH] coreutils-5.92 make_dir_parents regression
From: |
Jim Meyering |
Subject: |
Re: I: [PATCH] coreutils-5.92 make_dir_parents regression |
Date: |
Mon, 24 Oct 2005 15:45:51 +0200 |
"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;
}
}
}