bug-gnulib
[Top][All Lists]
Advanced

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

[PATCH] mkdir-p: Don't discard stat error


From: Niklas Hambüchen
Subject: [PATCH] mkdir-p: Don't discard stat error
Date: Fri, 14 Dec 2018 22:10:32 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.2

Until now, a stat() `errno` created by `mkdir -p` after `mkdir()` is dropped
silently, leading to very confusing error messages that hide the actual source
of the error:

`mkdir -p` can say

    cannot create directory ‘dir: File exists`

when indeed the the file exists, is a directory (shows `d` in `ls -l` as result
of getdents()'s `d_type` field), and the man page of `mkdir` says:

       -p, --parents
             no error if existing, make parent directories as needed

so `mkdir -p` shouldn't be able to say "file exists" when the directory exists.
Finally, the error message is inconsistent, because can give different error
messages (caused by the same cause) depending on whether you run `mkdir -p`
on the directory itself or a subdirectory of it (see also example below).

This happens especially frequently on FUSE mounts when the FUSE process mounted
at the directory was killed, but can also happen for other network mounts.

In general, the error of the `stat()` shouldn't be dropped, because it contains
the helpful error message that tells the user what the real problem is.

I suspect that the code in question called `stat()` mainly to distinguish
`S_ISDIR()` from from the other `S_*` macros, but forgot that `stat()`'s errno
can be very useful.

This commit fixes that.

You can reduce the example results below by running:

                mkdir /fuse-mountpoint
                sshfs localhost:/ /fuse-mountpoint
                killall -9 sshfs
                mkdir -p /fuse-mountpoint

Before:
Running `mkdir -p` on the mount dir gives inconsistent results (two different
errors depending on whether you mkdir the mount point or a subdirectory)
and an unhelpful message:

    $ mkdir -p /fuse-mountpoint
    mkdir: cannot create directory ‘/fuse-mountpoint’: File exists

    $ mkdir -p /fuse-mountpoint/subdir
    mkdir: cannot create directory ‘/fuse-mountpoint’: Transport endpoint is 
not connected

After:
Running `mkdir -p` on the mount dir gives consistent results and a helpful 
message:

    $ mkdir -p /fuse-mountpoint
    mkdir: cannot create directory ‘/fuse-mountpoint’: Transport endpoint is 
not connected
---
 lib/mkdir-p.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/lib/mkdir-p.c b/lib/mkdir-p.c
index c91b6c522..e0c8d597d 100644
--- a/lib/mkdir-p.c
+++ b/lib/mkdir-p.c
@@ -148,10 +148,19 @@ make_dir_parents (char *dir,
             {
               struct stat st;
               if (mkdir_errno == 0
-                  || (mkdir_errno != ENOENT && make_ancestor
-                      && stat (dir + prefix_len, &st) == 0
-                      && S_ISDIR (st.st_mode)))
-                return true;
+                  || (mkdir_errno != ENOENT && make_ancestor))
+                {
+                  if (stat (dir + prefix_len, &st) == 0
+                      && S_ISDIR (st.st_mode))
+                    return true;
+                  else
+                    {
+                      error (0, errno,
+                             _("cannot stat %s"),
+                             quote (dir + prefix_len));
+                      return false;
+                    }
+                }
             }
           else
             {
-- 
2.12.2




reply via email to

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