bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

mkdir-p patches from coreutils


From: Paul Eggert
Subject: mkdir-p patches from coreutils
Date: Thu, 22 Sep 2005 16:24:25 -0700
User-agent: Gnus/5.1007 (Gnus v5.10.7) Emacs/21.4 (gnu/linux)

I installed this:

2005-09-22  Paul Eggert  <address@hidden>

        * mkdir-p.c (ENOSYS): Define to EEXIST if not defined.
        (make_dir_parents): Treat ENOSYS like EEXIST.

        Improve quality of diagnostics on restore_cwd failure.
        * mkdir-p.h (make_dir): Remove.  All uses replaced by mkdir.
        (make_dir_parents): Last arg is now int * (for errno), not bool *.
        * mkdir-p.c (make_dir, make_dir_parents): Likewise.
        Rewrite "mkdir -p" algorithm to avoid the need for "stat"
        each time through the loop.  Do not diagnose restore_cwd failure;
        that is the caller's job (and perhaps the caller does not care).

        * mkdir-p.c (CLEANUP_CWD, CLEANUP): Remove.
        (make_dir_parents): Revamp to avoid need for CLEANUP_CWD, CLEANUP.
        If the file already exists but is not a directory, don't bother
        to try to make its parents.
        Close potential file descriptor leak if we can't chdir("/") (!).
        Don't always return true if chdir($PWD) fails; return true only
        if the requested action was done successfully (except for the
        chdir($PWD)).
        Don't log final directory unless we actually made it.
        Refactor to avoid duplicate code to fix up permissions.
        Don't attempt to fix up parent permissions if chdir($PWD) fails.

2005-09-22  Jim Meyering  <address@hidden>

        * mkdir-p.c (make_dir_parents): Don't let a failed chdir($PWD)
        stop us from restricting permissions of just-created absolute-named
        directories.
        * mkdir-p.c (CLEANUP_CWD): Return *true*, not false when failing
        to restore initial working directory.
        * mkdir-p.c (make_dir_parents): New parameter: different_working_dir,
        to tell caller if/when we change the working directory and are
        unable to return to the initial one.
        * mkdir-p.h (make_dir_parents): Update prototype.
        * mkdir-p.c (CLEANUP_CWD): Change one more `return 1' to
        `return false'.  This fixes a bug introduced on 2004-07-30.
        Assume HAVE_UNISTD_H, i.e., include <unistd.h> unconditionally.
        Assume HAVE_FCNTL_H (i.e., include <fcntl.h> unconditionally,
        and don't include <sys/file.h>).

Index: lib/mkdir-p.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/mkdir-p.h,v
retrieving revision 1.1
diff -p -u -r1.1 mkdir-p.h
--- lib/mkdir-p.h       2 Jun 2005 20:41:06 -0000       1.1
+++ lib/mkdir-p.h       22 Sep 2005 23:16:58 -0000
@@ -28,9 +28,5 @@ bool make_dir_parents (char const *argna
                       uid_t owner,
                       gid_t group,
                       bool preserve_existing,
-                      char const *verbose_fmt_string);
-
-bool make_dir (char const *dir,
-              char const *fulldir,
-              mode_t mode,
-              bool *created_dir_p);
+                      char const *verbose_fmt_string,
+                      int *cwd_errno);
Index: lib/mkdir-p.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/mkdir-p.c,v
retrieving revision 1.3
diff -p -u -r1.3 mkdir-p.c
--- lib/mkdir-p.c       19 Sep 2005 17:28:14 -0000      1.3
+++ lib/mkdir-p.c       22 Sep 2005 23:16:58 -0000
@@ -30,9 +30,7 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#if HAVE_UNISTD_H
-# include <unistd.h>
-#endif
+#include <unistd.h>
 
 #include <stdlib.h>
 #include <errno.h>
@@ -47,89 +45,11 @@
 #include "quote.h"
 #include "stat-macros.h"
 
-#define WX_USR (S_IWUSR | S_IXUSR)
-
-#define CLEANUP_CWD                                    \
-  do                                                   \
-    {                                                  \
-      /* We're done operating on basename_dir.         \
-        Restore working directory.  */                 \
-      if (do_chdir)                                    \
-       {                                               \
-         if (restore_cwd (&cwd) != 0)                  \
-           {                                           \
-             int _saved_errno = errno;                 \
-             error (0, errno,                          \
-               _("failed to return to initial working directory")); \
-             free_cwd (&cwd);                          \
-             errno = _saved_errno;                     \
-             return 1;                                 \
-           }                                           \
-         free_cwd (&cwd);                              \
-       }                                               \
-    }                                                  \
-  while (0)
-
-#define CLEANUP                                                \
-  do                                                   \
-    {                                                  \
-      umask (oldmask);                                 \
-      CLEANUP_CWD;                                     \
-    }                                                  \
-  while (0)
-
-/* Attempt to create directory DIR (aka FULLDIR) with the specified MODE.
-   If CREATED_DIR_P is non-NULL, set *CREATED_DIR_P if this
-   function creates DIR and clear it otherwise.  Give a diagnostic and
-   return false if DIR cannot be created or cannot be determined to
-   exist already.  Use FULLDIR in any diagnostic, not DIR.
-   Note that if DIR already exists, this function returns true
-   (indicating success) and clears *CREATED_DIR_P.  */
-
-bool
-make_dir (char const *dir, char const *fulldir, mode_t mode,
-         bool *created_dir_p)
-{
-  bool ok = true;
-  bool created_dir;
-
-  created_dir = (mkdir (dir, mode) == 0);
-
-  if (!created_dir)
-    {
-      struct stat stats;
-      int saved_errno = errno;
-
-      /* The mkdir and stat calls below may appear to be reversed.
-        They are not.  It is important to call mkdir first and then to
-        call stat (to distinguish the three cases) only if mkdir fails.
-        The alternative to this approach is to `stat' each directory,
-        then to call mkdir if it doesn't exist.  But if some other process
-        were to create the directory between the stat & mkdir, the mkdir
-        would fail with EEXIST.  */
-
-      if (stat (dir, &stats))
-       {
-         error (0, saved_errno, _("cannot create directory %s"),
-                quote (fulldir));
-         ok = false;
-       }
-      else if (!S_ISDIR (stats.st_mode))
-       {
-         error (0, 0, _("%s exists but is not a directory"), quote (fulldir));
-         ok = false;
-       }
-      else
-       {
-         /* DIR (aka FULLDIR) already exists and is a directory. */
-       }
-    }
-
-  if (created_dir_p)
-    *created_dir_p = created_dir;
+#ifndef ENOSYS
+# define ENOSYS EEXIST
+#endif
 
-  return ok;
-}
+#define WX_USR (S_IWUSR | S_IXUSR)
 
 /* Ensure that the directory ARG exists.
 
@@ -145,8 +65,14 @@ make_dir (char const *dir, char const *f
    If PRESERVE_EXISTING is true and ARG is an existing directory,
    then do not attempt to set its permissions and ownership.
 
-   Return true iff ARG exists as a directory with the proper
-   ownership and permissions when done.  */
+   Set *CWD_ERRNO to a (nonzero) error number if this
+   function has changed the current working directory and is unable to
+   restore it to its initial state.  Do not change
+   *CWD_ERRNO otherwise.
+
+   Return true iff ARG exists as a directory with the proper ownership
+   and permissions when done.  Note that this function returns true
+   even when it fails to return to the initial working directory.  */
 
 bool
 make_dir_parents (char const *arg,
@@ -155,24 +81,45 @@ make_dir_parents (char const *arg,
                  uid_t owner,
                  gid_t group,
                  bool preserve_existing,
-                 char const *verbose_fmt_string)
+                 char const *verbose_fmt_string,
+                 int *cwd_errno)
 {
   struct stat stats;
   bool retval = true;
+  bool do_chdir = false;       /* Whether to chdir before each mkdir.  */
+  struct saved_cwd cwd;
+  bool cwd_problem = false;
+  char const *fixup_permissions_dir = NULL;
+  char const *full_dir = arg;
+
+  struct ptr_list
+  {
+    char *dirname_end;
+    struct ptr_list *next;
+  };
+  struct ptr_list *leading_dirs = NULL;
 
-  if (stat (arg, &stats) != 0)
+  if (stat (arg, &stats) == 0)
+    {
+      if (! S_ISDIR (stats.st_mode))
+       {
+         error (0, 0, _("%s exists but is not a directory"), quote (arg));
+         return false;
+       }
+
+      if (!preserve_existing)
+       fixup_permissions_dir = arg;
+    }
+  else if (errno != ENOENT || !*arg)
+    {
+      error (0, errno, "%s", quote (arg));
+      return false;
+    }
+  else
     {
       char *slash;
       mode_t tmp_mode;         /* Initial perms for leading dirs.  */
       bool re_protect;         /* Should leading dirs be unwritable? */
-      struct ptr_list
-      {
-       char *dirname_end;
-       struct ptr_list *next;
-      };
-      struct ptr_list *p, *leading_dirs = NULL;
-      bool do_chdir;           /* Whether to chdir before each mkdir.  */
-      struct saved_cwd cwd;
       char *basename_dir;
       char *dir;
 
@@ -183,6 +130,7 @@ make_dir_parents (char const *arg,
       dir = (char *) alloca (strlen (arg) + 1);
       strcpy (dir, arg);
       strip_trailing_slashes (dir);
+      full_dir = dir;
 
       /* If leading directories shouldn't be writable or executable,
         or should have set[ug]id or sticky bits set and we are setting
@@ -213,7 +161,10 @@ make_dir_parents (char const *arg,
             file name starts with exactly two slashes.  */
          char const *root = "//" + (dir[1] != '/' || dir[2] == '/');
          if (chdir (root) != 0)
-           do_chdir = false;
+           {
+             free_cwd (&cwd);
+             do_chdir = false;
+           }
        }
 
       slash = dir;
@@ -222,10 +173,8 @@ make_dir_parents (char const *arg,
       while (*slash == '/')
        slash++;
 
-      while (1)
+      while (true)
        {
-         bool newly_created_dir;
-
          /* slash points to the leftmost unprocessed component of dir.  */
          basename_dir = slash;
 
@@ -239,13 +188,7 @@ make_dir_parents (char const *arg,
            basename_dir = dir;
 
          *slash = '\0';
-         if (! make_dir (basename_dir, dir, tmp_mode, &newly_created_dir))
-           {
-             CLEANUP;
-             return false;
-           }
-
-         if (newly_created_dir)
+         if (mkdir (basename_dir, tmp_mode) == 0)
            {
              if (verbose_fmt_string)
                error (0, 0, verbose_fmt_string, quote (dir));
@@ -259,8 +202,8 @@ make_dir_parents (char const *arg,
                {
                  error (0, errno, _("cannot change owner and/or group of %s"),
                         quote (dir));
-                 CLEANUP;
-                 return false;
+                 retval = false;
+                 break;
                }
 
              if (re_protect)
@@ -272,22 +215,37 @@ make_dir_parents (char const *arg,
                  leading_dirs = new;
                }
            }
+         else if (errno == EEXIST || errno == ENOSYS)
+           {
+             /* A file is already there.  Perhaps it is a directory.
+                If not, it will be diagnosed later.
+
+                The ENOSYS is for Solaris 8 NFS clients, which can
+                fail with errno == ENOSYS if mkdir is invoked on an
+                NFS mount point.  */
+           }
+         else
+           {
+             error (0, errno, _("cannot create directory %s"), quote (dir));
+             retval = false;
+             break;
+           }
 
          /* If we were able to save the initial working directory,
             then we can use chdir to change into each directory before
             creating an entry in that directory.  This avoids making
-            stat and mkdir process O(n^2) file name components.  */
+            mkdir process O(n^2) file name components.  */
          if (do_chdir && chdir (basename_dir) < 0)
            {
              error (0, errno, _("cannot chdir to directory %s"),
                     quote (dir));
-             CLEANUP;
-             return false;
+             retval = false;
+             break;
            }
 
          *slash++ = '/';
 
-         /* Avoid unnecessary calls to `stat' when given
+         /* Avoid unnecessary calls to mkdir when given
             file names containing multiple adjacent slashes.  */
          while (*slash == '/')
            slash++;
@@ -301,26 +259,40 @@ make_dir_parents (char const *arg,
 
       /* We're done making leading directories.
         Create the final component of the file name.  */
-
-      if (! make_dir (basename_dir, dir, mode, NULL))
+      if (retval)
        {
-         CLEANUP;
-         return false;
+         if (mkdir (basename_dir, mode) != 0)
+           {
+             error (0, errno, _("cannot create directory %s"), quote (dir));
+             retval = false;
+           }
+         else
+           {
+             if (verbose_fmt_string)
+               error (0, 0, verbose_fmt_string, quote (dir));
+             fixup_permissions_dir = basename_dir;
+           }
        }
+    }
 
-      if (verbose_fmt_string != NULL)
-       error (0, 0, verbose_fmt_string, quote (dir));
+  if (fixup_permissions_dir)
+    {
+      /* chown must precede chmod because on some systems,
+        chown clears the set[ug]id bits for non-superusers,
+        resulting in incorrect permissions.
+        On System V, users can give away files with chown and then not
+        be able to chmod them.  So don't give files away.  */
 
       if (owner != (uid_t) -1 || group != (gid_t) -1)
        {
-         if (chown (basename_dir, owner, group)
+         if (chown (fixup_permissions_dir, owner, group) != 0
 #ifdef AFS
              && errno != EPERM
 #endif
              )
            {
              error (0, errno, _("cannot change owner and/or group of %s"),
-                    quote (dir));
+                    quote (full_dir));
              retval = false;
            }
        }
@@ -330,67 +302,35 @@ make_dir_parents (char const *arg,
         required to honor only the file permission bits.  In particular,
         it need not honor the `special' bits, so if MODE includes any
         special bits, set them here.  */
-      if ((mode & ~S_IRWXUGO)
-         && chmod (basename_dir, mode))
+      if ((mode & ~S_IRWXUGO) && chmod (fixup_permissions_dir, mode) != 0)
        {
          error (0, errno, _("cannot change permissions of %s"),
-                quote (dir));
+                quote (full_dir));
          retval = false;
        }
-
-      CLEANUP_CWD;
-
-      /* If the mode for leading directories didn't include owner "wx"
-        privileges, we have to reset their protections to the correct
-        value.  */
-      for (p = leading_dirs; p != NULL; p = p->next)
-       {
-         *(p->dirname_end) = '\0';
-         if (chmod (dir, parent_mode) != 0)
-           {
-             error (0, errno, _("cannot change permissions of %s"),
-                    quote (dir));
-             retval = false;
-           }
-       }
     }
-  else
-    {
-      /* We get here if the file already exists.  */
-
-      char const *dir = arg;
 
-      if (!S_ISDIR (stats.st_mode))
+  if (do_chdir)
+    {
+      if (restore_cwd (&cwd) != 0)
        {
-         error (0, 0, _("%s exists but is not a directory"), quote (dir));
-         return false;
+         *cwd_errno = errno;
+         cwd_problem = true;
        }
+      free_cwd (&cwd);
+    }
 
-      if (!preserve_existing)
+  /* If the mode for leading directories didn't include owner "wx"
+     privileges, reset their protections to the correct value.  */
+  for (; leading_dirs != NULL; leading_dirs = leading_dirs->next)
+    {
+      leading_dirs->dirname_end[0] = '\0';
+      if ((cwd_problem && *full_dir != '/')
+         || chmod (full_dir, parent_mode) != 0)
        {
-         /* chown must precede chmod because on some systems,
-            chown clears the set[ug]id bits for non-superusers,
-            resulting in incorrect permissions.
-            On System V, users can give away files with chown and then not
-            be able to chmod them.  So don't give files away.  */
-
-         if ((owner != (uid_t) -1 || group != (gid_t) -1)
-             && chown (dir, owner, group)
-#ifdef AFS
-             && errno != EPERM
-#endif
-             )
-           {
-             error (0, errno, _("cannot change owner and/or group of %s"),
-                    quote (dir));
-             retval = false;
-           }
-         if (chmod (dir, mode) != 0)
-           {
-             error (0, errno, _("cannot change permissions of %s"),
-                                quote (dir));
-             retval = false;
-           }
+         error (0, (cwd_problem ? 0 : errno),
+                _("cannot change permissions of %s"), quote (full_dir));
+         retval = false;
        }
     }
 
Index: m4/mkdir-p.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/mkdir-p.m4,v
retrieving revision 1.1
diff -p -u -r1.1 mkdir-p.m4
--- m4/mkdir-p.m4       2 Jun 2005 20:41:08 -0000       1.1
+++ m4/mkdir-p.m4       22 Sep 2005 23:16:58 -0000
@@ -1,4 +1,4 @@
-# mkdir-p.m4 serial 8
+# mkdir-p.m4 serial 9
 dnl Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -11,6 +11,5 @@ AC_DEFUN([gl_MKDIR_PARENTS],
 
   dnl Prerequisites of lib/mkdir-p.c.
   AC_REQUIRE([AC_FUNC_ALLOCA])
-  AC_CHECK_HEADERS_ONCE(unistd.h)
   AC_REQUIRE([gl_AFS])
 ])




reply via email to

[Prev in Thread] Current Thread [Next in Thread]