GNU bug report logs - #41001
mkdir: cannot create directory ‘test’: File exists

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41001 in the body.
You can then email your comments to 41001 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: bug-coreutils <at> gnu.org
Subject: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 16:06:36 +0100

Hello!

Can this error message be clarified? The directory already exists, it is 
not a file.

lib/mkdir-p.c:200 contains this line of code that triggers below:-

error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));

As it's easy enough to know that the reason mkdir fails is because 
'test' a directory that already exists.

Easy enough to check with stat() and S_ISDIR(sb.st_mode)

Can this be changed? Maybe I can make a patch for it.
Jonny



$ mkdir test
$ mkdir test
mkdir: cannot create directory ‘test’: File exists
$ mkdir --version
mkdir (GNU coreutils) 8.28
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David MacKenzie.
$

Message #12 received at 41001-done <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Jonny Grant <jg <at> jguk.org>, 41001-done <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 10:54:37 -0500

tag 41001 notabug
thanks

On 5/1/20 10:06 AM, Jonny Grant wrote:
> Hello!
> 
> Can this error message be clarified? The directory already exists, it is 
> not a file.

By one definition, a directory _is_ a file, just with different 
semantics (in the same way a block device, character device, symlink, 
fifo, or socket can also be a file).  It is not a regular file, but "a 
file" is any entry stored in a directory, and as subdirectories are 
stored in a directory, they count as files.

> 
> lib/mkdir-p.c:200 contains this line of code that triggers below:-
> 
> error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));

The error message in question is coming from libc's strerror() function; 
if you want a different error message for EEXIST, you'll have to 
convince glibc to update their error string tables.  It is not something 
that coreutils directly controls.  And while we could indeed output a 
custom message instead of using strerror(), that would confuse people 
who have grown used to the strerror() message.

> 
> As it's easy enough to know that the reason mkdir fails is because 
> 'test' a directory that already exists.
> 
> Easy enough to check with stat() and S_ISDIR(sb.st_mode)
> 
> Can this be changed? Maybe I can make a patch for it.
> Jonny
> 
> 
> 
> $ mkdir test
> $ mkdir test
> mkdir: cannot create directory ‘test’: File exists

If nothing else, you may want to consider using 'mkdir -p test', which 
specifically checks if the reason for an EEXIST failure is because the 
directory already exists.  Once you do that, you don't need to patch 
coreutils.

Thus, I'm closing this as not a bug; but feel free to respond further 
with any more comments on the topic.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Message #15 received at 41001-done <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Eric Blake <eblake <at> redhat.com>, 41001-done <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 17:16:13 +0100

On 01/05/2020 16:54, Eric Blake wrote:
> tag 41001 notabug
> thanks
> 
> On 5/1/20 10:06 AM, Jonny Grant wrote:
>> Hello!
>>
>> Can this error message be clarified? The directory already exists, it 
>> is not a file.
> 
> By one definition, a directory _is_ a file, just with different 
> semantics (in the same way a block device, character device, symlink, 
> fifo, or socket can also be a file).  It is not a regular file, but "a 
> file" is any entry stored in a directory, and as subdirectories are 
> stored in a directory, they count as files.
> 
>>
>> lib/mkdir-p.c:200 contains this line of code that triggers below:-
>>
>> error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));
> 
> The error message in question is coming from libc's strerror() function; 
> if you want a different error message for EEXIST, you'll have to 
> convince glibc to update their error string tables.  It is not something 
> that coreutils directly controls.  And while we could indeed output a 
> custom message instead of using strerror(), that would confuse people 
> who have grown used to the strerror() message.
> 
>>
>> As it's easy enough to know that the reason mkdir fails is because 
>> 'test' a directory that already exists.
>>
>> Easy enough to check with stat() and S_ISDIR(sb.st_mode)
>>
>> Can this be changed? Maybe I can make a patch for it.
>> Jonny
>>
>>
>>
>> $ mkdir test
>> $ mkdir test
>> mkdir: cannot create directory ‘test’: File exists
> 
> If nothing else, you may want to consider using 'mkdir -p test', which 
> specifically checks if the reason for an EEXIST failure is because the 
> directory already exists.  Once you do that, you don't need to patch 
> coreutils.
> 
> Thus, I'm closing this as not a bug; but feel free to respond further 
> with any more comments on the topic.
> 


Hello Eric!

Yes, I can use mkdir -p as a workaround. Yes, I know everything is a 
file ultimately. However, most software does not call directories files 
as the routine behaviour for users.

'rm' and 'cat' are part of coreutils, that has output as I expected it.


$ rm test
rm: cannot remove 'test': Is a directory

$ cat test
cat: test: Is a directory





I'm certain glibc won't/can't change EEXIST, as it is POSIX.



$ strings test
strings: Warning: 'test' is a directory

$ vim test
"test" is a directory


on a tangent:
I see the cat output doesn't put the name in quotes, perhaps it should?

cat: 'test': Is a directory


Regards
Jonny

Message #18 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 11:07:48 -0700

On 5/1/20 9:16 AM, Jonny Grant wrote:
> rm: cannot remove 'test': Is a directory

That's because rm used unlink which failed with EISDIR, which is a different
error number.

Consider this example:

    $ >d # Create an empty regular file.
    $ mkdir d
    mkdir: cannot create directory ‘d’: File exists

Here the system call mkdir("d", 0777) failed with errno == EEXIST (File exists).
Presumably you wouldn't object to the diagnostic here because d is a regular
file, not a directory. But the mkdir system call fails in exactly the same way
if d is a directory, so the error message is the same in both cases.

Directories are files, so the error message is correct even if it confused you.
I don't see any portable and efficient way to make the diagnostic less confusing
for you, without also making diagnostic incorrect in some other scenarios (such
as the scenario described above).

Message #21 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 21:21:42 +0100

On 01/05/2020 19:07, Paul Eggert wrote:
> On 5/1/20 9:16 AM, Jonny Grant wrote:
>> rm: cannot remove 'test': Is a directory
> 
> That's because rm used unlink which failed with EISDIR, which is a different
> error number.

yes, the fix pretty trivial for mkdir as you highlight EISDIR:
stat(), S_ISDIR(sb.st_mode), and set errno to EISDIR or output 
strerror(EISDIR)

$ mkdir test
mkdir: cannot create directory ‘test’: Is a directory

> Consider this example:
> 
>      $ >d # Create an empty regular file.
>      $ mkdir d
>      mkdir: cannot create directory ‘d’: File exists
> 
> Here the system call mkdir("d", 0777) failed with errno == EEXIST (File exists).
> Presumably you wouldn't object to the diagnostic here because d is a regular
> file, not a directory. But the mkdir system call fails in exactly the same way
> if d is a directory, so the error message is the same in both cases.

Exactly, UNIX didn't create separate errno for files and directories, 
it's the same limitation with ENOENT. As a developer, we handle it 
ourselves, as it's easy enough to call stat() like other package 
maintainers do, as you can see in binutils.

> Directories are files, so the error message is correct even if it confused you.
> I don't see any portable and efficient way to make the diagnostic less confusing
> for you, without also making diagnostic incorrect in some other scenarios (such
> as the scenario described above).

Feels like the fix I already proposed does not have any incorrect impact 
in the other scenario you describe? Do correct me if I am missing something.

Yes, as a developer I know everything is actually a file, but users 
don't. Users will call it a folder, or a directory. I didn't go over 
UNIX everything-is-a-file in my bug report because everyone here knows 
already.

This one is an simple fix, but it's clear no one wants to introduce the 
change, no worries.

Cheers, Jonny

Message #24 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Fri, 1 May 2020 13:32:19 -0700

On 5/1/20 1:21 PM, Jonny Grant wrote:
> yes, the fix pretty trivial for mkdir as you highlight EISDIR:
> stat(), S_ISDIR(sb.st_mode), and set errno to EISDIR or output strerror(EISDIR)

That would introduce a race condition, and wouldn't behave correctly if some
other process changes the destination from a regular file to a directory between
the time we call mkdir and the time that we call stat.

Message #27 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sat, 2 May 2020 14:26:35 +0100

On 01/05/2020 21:32, Paul Eggert wrote:
> On 5/1/20 1:21 PM, Jonny Grant wrote:
>> yes, the fix pretty trivial for mkdir as you highlight EISDIR:
>> stat(), S_ISDIR(sb.st_mode), and set errno to EISDIR or output strerror(EISDIR)
> 
> That would introduce a race condition, and wouldn't behave correctly if some
> other process changes the destination from a regular file to a directory between
> the time we call mkdir and the time that we call stat.

Paul,

If developers have race conditions in their shell scripts - mkdir error 
string in the message after the colon in the output saying 
file/directory is the least of the developers' problems.

mkdir() returning EEXIST only indicates the pathname exists.

Maybe call stat() before calling mkdir() to check nothing there. It's 
more a question of doing something appropriate.

Personally I doubt POSIX will ever be updated to have more errno errors 
that distinguish between files and directories for ENOENT and EEXIST due 
to people's fears about compatibility when APIs are updated.

Cheers, Jonny

Message #30 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sat, 2 May 2020 12:47:49 -0700

On 5/2/20 6:26 AM, Jonny Grant wrote:
> If developers have race conditions in their shell scripts

I've personally fixed a bug in the GNU mkdir command that was triggered by such
races. Core utilities should be reliable even when these races are happening.

Message #33 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sat, 2 May 2020 23:41:43 +0100

On 02/05/2020 20:47, Paul Eggert wrote:
> On 5/2/20 6:26 AM, Jonny Grant wrote:
>> If developers have race conditions in their shell scripts
> 
> I've personally fixed a bug in the GNU mkdir command that was triggered by such
> races. Core utilities should be reliable even when these races are happening.

Is a more accurate strerror considered unreliable?

Current:
mkdir: cannot create directory ‘test’: File exists

Proposed:
mkdir: cannot create directory ‘test’: Is a directory

Cheers, Jonny

Message #36 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sat, 2 May 2020 16:13:03 -0700

On 5/2/20 3:41 PM, Jonny Grant wrote:
> Is a more accurate strerror considered unreliable?
> 
> Current:
> mkdir: cannot create directory ‘test’: File exists
> 
> Proposed:
> mkdir: cannot create directory ‘test’: Is a directory

I don't understand this comment. As I understand it you're proposing a change to
the mkdir command not a change to the strerror library function, and the change
you're proposing would introduce a race condition to the mkdir command.

A better fix would be to change the mkdir system call so that it sets errno to
EISDIR in this situation. This would fix not only the mkdir utility, but also
lots of other programs; and it wouldn't introduce a race condition. So if you're
interested in getting the problem fixed, I suggest that you propose such a
change to the Linux kernel developers.

Message #39 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 41001 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sun, 3 May 2020 14:02:29 +0100

Hi!

On 03/05/2020 00:13, Paul Eggert wrote:
> On 5/2/20 3:41 PM, Jonny Grant wrote:
>> Is a more accurate strerror considered unreliable?
>>
>> Current:
>> mkdir: cannot create directory ‘test’: File exists
>>
>> Proposed:
>> mkdir: cannot create directory ‘test’: Is a directory
> 
> I don't understand this comment. As I understand it you're proposing a change to
> the mkdir command not a change to the strerror library function, and the change
> you're proposing would introduce a race condition to the mkdir command.

As the mkdir error returned to the shell is the same, I don't feel the 
difference between the words "File exists" and "Is a directory" on the 
terminal can be considered a race condition.


You're right, there will be a race condition where two processes are 
both creating and deleting the same files. Any software which is 
creating and deleting the same directories in parallel will encounter a 
multitude of errors - all bets are off.


> A better fix would be to change the mkdir system call so that it sets errno to
> EISDIR in this situation. This would fix not only the mkdir utility, but also
> lots of other programs; and it wouldn't introduce a race condition. So if you're
> interested in getting the problem fixed, I suggest that you propose such a
> change to the Linux kernel developers.

Yes, if Linux kernel developers would deviate from POSIX.  I emailed 
linux-ext4 <at> vger.kernel.org the lines of code to change.

I'm not confident it will get in, even harder to get into POSIX I expect.

ext4_match() is what would need to be updated to check if an entry is a 
directory

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/ext4/namei.c


Cheers, Jonny

Message #42 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Bob Proulx <bob <at> proulx.com>
To: Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Mon, 4 May 2020 13:32:32 -0600

Jonny Grant wrote:
> Paul Eggert wrote:
> > Jonny Grant wrote:
> > > Is a more accurate strerror considered unreliable?
> > > 
> > > Current:
> > > mkdir: cannot create directory ‘test’: File exists
> > > 
> > > Proposed:
> > > mkdir: cannot create directory ‘test’: Is a directory
> > 
> > I don't understand this comment. As I understand it you're proposing a change to
> > the mkdir command not a change to the strerror library function, and the change
> > you're proposing would introduce a race condition to the mkdir command.
> 
> As the mkdir error returned to the shell is the same, I don't feel the
> difference between the words "File exists" and "Is a directory" on the
> terminal can be considered a race condition.

I read the message thread carefully and the proposal was to add an
additional non-atomic stat(2) call to the logic.  That sets up the
race condition.

The difference in the words of the error string is not the race
condition.  The race condition is created when trying to stat(2) the
file to see why it failed.  That can only be done as a separate
action.  That cannot be an atomic operation.  That can only create a
race condition.

For the low level utilities it is almost always a bad idea to layer in
additional system calls that are not otherwise there.  Doing so almost
always creates additional bugs.  And then there will be new bug
reports about those problems.  And those will be completely valid.

Try this experiment on your own.

  /tmp$ strace -e trace=mkdir mkdir foodir1
  mkdir("foodir1", 0777)                  = 0
  +++ exited with 0 +++

  /tmp$ strace -e trace=mkdir mkdir foodir1
  mkdir("foodir1", 0777)                  = -1 EEXIST (File exists)
  mkdir: cannot create directory ‘foodir1’: File exists
  +++ exited with 1 +++

The first mkdir("foodir1", 0777) call succeeded.  The second
mkdir("foodir1", 0777) call fail, returned -1, set errno = EEXIST,
EEXIST is the error number for "File exists".

Note that this output line:

  mkdir("foodir1", 0777)                  = -1 EEXIST (File exists)

That line was entirely reported by the 'strace' command and is not any
code related to the Coreutils mkdir command.  The strace command
reported the same "File exists" message as mkdir did later, due to the
EEXIST error code.

Let's try the same experiment with a file.  And also with a pipe and a
character device too.

  /tmp$ touch file1

  /tmp$ strace -e trace=mkdir mkdir file1
  mkdir("file1", 0777)                    = -1 EEXIST (File exists)
  mkdir: cannot create directory ‘file1’: File exists
  +++ exited with 1 +++

  /tmp$ mkfifo fifo1

  strace -e trace=mkdir mkdir fifo1
  mkdir("fifo1", 0777)                    = -1 EEXIST (File exists)
  mkdir: cannot create directory ‘fifo1’: File exists
  +++ exited with 1 +++

  /tmp$ sudo mknod char1 c 5 0

  /tmp$ strace -e trace=mkdir mkdir char1
  mkdir("char1", 0777)                    = -1 EEXIST (File exists)
  mkdir: cannot create directory ‘char1’: File exists
  +++ exited with 1 +++

And so we see that the kernel is returning the same EEXIST error code
for *all* cases where a file previously exists.  And it is correct
because all of those are files.  Because directories are files, pipes
are files, and files are files.  Everything is a file.  Therefore
EEXIST is a correct error message.

In order to correctly change the message being reported the change
should be made in the kernel so that the kernel, which has the
information at that time atomically, could report an error providing
more detail than simply EEXIST.

You have proposed that mkdir add a stat(2) system call to extract this
additional information.

> as it's easy enough to call stat() like other package maintainers
> do, as you can see in binutils.

*That* stat() addition creates the race condition.  Adding a stat()
call cannot be done atomically.

It would need to be done either before the mkdir(), after the mkdir(),
or both before and after.  Let's see how that can go wrong.  Let's say
we stat(), does not exist, we continue with mkdir(), fails with EEXIST
because another process got there first.  So then we stat() again and
by that time the other process has already finished processing and
removed the directory again.  A system call trace would look like
this.

  lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)
  mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
  lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)

Okay.  That's confusing.  The only value in hand being EEXIST then
that is the error to be reported.  If this were repeated many times
then sometimes we would catch it as an actual directory.

  lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)
  mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
  lstat("foodir1", {st_mode=S_IFDIR|0775, st_size=40, ...}) = 0

In that case the proposal is to report it as EISDIR.  If we were to
set up two processes coordinating using a directory as a semaphore and
printing out the errors then we would see a stream of output that is
sometimes the creation of the directory, sometimes the failure since a
directory already exists, and sometimes a failure because it is a
file.  Race condition.  That would be extremely confusing!  I assure
you the valid bug reports would be swift and correctly merciless!

And there is another possibility that is also bad too.  We stat(),
does not exist, we continue with mkdir(), fails with EEXIST because
another process got there first creating it as a file.  So then we
stat() again and by that time the other process has removed the file
and replaced it with a fifo device node.

  lstat("file1", 0x7ffdedf9aca0)         = -1 ENOENT (No such file or directory)
  mkdir("file1", 0777)                   = -1 EEXIST (File exists)
  lstat("fifo1", {st_mode=S_IFIFO|0664, st_size=0, ...}) = 0

At that point the reason the mkdir actually failed was that it was a
file but the second stat() would have a report that it was a fifo
device which would be incorrect.  Race condition.

For the low level utilities it is almost always best to remain close
to the kernel and report the actual error reported by the kernel.
Constructing a layer in the low level utilities is almost always a bad
idea.

However in your own shell scripts if you wish to construct exactly
this same layer that you are proposing then that is your prerogative.
But it would open your program up to valid bug reports for when the
race condition tripped one of the problem cases.

> You're right, there will be a race condition where two processes are both
> creating and deleting the same files. Any software which is creating and
> deleting the same directories in parallel will encounter a multitude of
> errors - all bets are off.

I am glad that you agree.  That is exactly why adding non-atomic
operations to the low level utilities creating those race coditions is
a bad idea.

> > A better fix would be to change the mkdir system call so that it sets errno to
> > EISDIR in this situation. This would fix not only the mkdir utility, but also
> > lots of other programs; and it wouldn't introduce a race condition. So if you're
> > interested in getting the problem fixed, I suggest that you propose such a
> > change to the Linux kernel developers.
> 
> Yes, if Linux kernel developers would deviate from POSIX.  I emailed
> linux-ext4 <at> vger.kernel.org the lines of code to change.

I am glad to see that you are following up in the right place.  That
place being in the kernel.  It can't be addressed correctly in user space.

> I'm not confident it will get in, even harder to get into POSIX I expect.
> 
> ext4_match() is what would need to be updated to check if an entry is a
> directory
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/ext4/namei.c

ext4?  But what if I am using xfs?  Does this then behave differently
on ext4 versus ext3 versus xfs versus nfs4 versus...  Well...  You get
the idea.

Sometimes the only way to win is not to play.  Is the current EEXIST
really so bad that we would create additional problems just to avoid
it?  Simplest is almost always best.

Bob

Message #45 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Bob Proulx <bob <at> proulx.com>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Tue, 5 May 2020 19:36:32 +0100

On 04/05/2020 20:32, Bob Proulx wrote:
> Jonny Grant wrote:
>> Paul Eggert wrote:
>>> Jonny Grant wrote:
>>>> Is a more accurate strerror considered unreliable?
>>>>
>>>> Current:
>>>> mkdir: cannot create directory ‘test’: File exists
>>>>
>>>> Proposed:
>>>> mkdir: cannot create directory ‘test’: Is a directory
>>>
>>> I don't understand this comment. As I understand it you're proposing a change to
>>> the mkdir command not a change to the strerror library function, and the change
>>> you're proposing would introduce a race condition to the mkdir command.
>>
>> As the mkdir error returned to the shell is the same, I don't feel the
>> difference between the words "File exists" and "Is a directory" on the
>> terminal can be considered a race condition.
> 
> I read the message thread carefully and the proposal was to add an
> additional non-atomic stat(2) call to the logic.  That sets up the
> race condition.
> 
> The difference in the words of the error string is not the race
> condition.  The race condition is created when trying to stat(2) the
> file to see why it failed.  That can only be done as a separate
> action.  That cannot be an atomic operation.  That can only create a
> race condition.
> 
> For the low level utilities it is almost always a bad idea to layer in
> additional system calls that are not otherwise there.  Doing so almost
> always creates additional bugs.  And then there will be new bug
> reports about those problems.  And those will be completely valid.
> 
> Try this experiment on your own.
> 
>    /tmp$ strace -e trace=mkdir mkdir foodir1
>    mkdir("foodir1", 0777)                  = 0
>    +++ exited with 0 +++
> 
>    /tmp$ strace -e trace=mkdir mkdir foodir1
>    mkdir("foodir1", 0777)                  = -1 EEXIST (File exists)
>    mkdir: cannot create directory ‘foodir1’: File exists
>    +++ exited with 1 +++
> 
> The first mkdir("foodir1", 0777) call succeeded.  The second
> mkdir("foodir1", 0777) call fail, returned -1, set errno = EEXIST,
> EEXIST is the error number for "File exists".
> 
> Note that this output line:
> 
>    mkdir("foodir1", 0777)                  = -1 EEXIST (File exists)
> 
> That line was entirely reported by the 'strace' command and is not any
> code related to the Coreutils mkdir command.  The strace command
> reported the same "File exists" message as mkdir did later, due to the
> EEXIST error code.
> 
> Let's try the same experiment with a file.  And also with a pipe and a
> character device too.
> 
>    /tmp$ touch file1
> 
>    /tmp$ strace -e trace=mkdir mkdir file1
>    mkdir("file1", 0777)                    = -1 EEXIST (File exists)
>    mkdir: cannot create directory ‘file1’: File exists
>    +++ exited with 1 +++
> 
>    /tmp$ mkfifo fifo1
> 
>    strace -e trace=mkdir mkdir fifo1
>    mkdir("fifo1", 0777)                    = -1 EEXIST (File exists)
>    mkdir: cannot create directory ‘fifo1’: File exists
>    +++ exited with 1 +++
> 
>    /tmp$ sudo mknod char1 c 5 0
> 
>    /tmp$ strace -e trace=mkdir mkdir char1
>    mkdir("char1", 0777)                    = -1 EEXIST (File exists)
>    mkdir: cannot create directory ‘char1’: File exists
>    +++ exited with 1 +++
> 
> And so we see that the kernel is returning the same EEXIST error code
> for *all* cases where a file previously exists.  And it is correct
> because all of those are files.  Because directories are files, pipes
> are files, and files are files.  Everything is a file.  Therefore
> EEXIST is a correct error message.
> 
> In order to correctly change the message being reported the change
> should be made in the kernel so that the kernel, which has the
> information at that time atomically, could report an error providing
> more detail than simply EEXIST.
> 
> You have proposed that mkdir add a stat(2) system call to extract this
> additional information.
> 
>> as it's easy enough to call stat() like other package maintainers
>> do, as you can see in binutils.
> 
> *That* stat() addition creates the race condition.  Adding a stat()
> call cannot be done atomically.
> 
> It would need to be done either before the mkdir(), after the mkdir(),
> or both before and after.  Let's see how that can go wrong.  Let's say
> we stat(), does not exist, we continue with mkdir(), fails with EEXIST
> because another process got there first.  So then we stat() again and
> by that time the other process has already finished processing and
> removed the directory again.  A system call trace would look like
> this.
> 
>    lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)
>    mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
>    lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)
> 
> Okay.  That's confusing.  The only value in hand being EEXIST then
> that is the error to be reported.  If this were repeated many times
> then sometimes we would catch it as an actual directory.
> 
>    lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or directory)
>    mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
>    lstat("foodir1", {st_mode=S_IFDIR|0775, st_size=40, ...}) = 0
> 
> In that case the proposal is to report it as EISDIR.  If we were to
> set up two processes coordinating using a directory as a semaphore and
> printing out the errors then we would see a stream of output that is
> sometimes the creation of the directory, sometimes the failure since a
> directory already exists, and sometimes a failure because it is a
> file.  Race condition.  That would be extremely confusing!  I assure
> you the valid bug reports would be swift and correctly merciless!
> 
> And there is another possibility that is also bad too.  We stat(),
> does not exist, we continue with mkdir(), fails with EEXIST because
> another process got there first creating it as a file.  So then we
> stat() again and by that time the other process has removed the file
> and replaced it with a fifo device node.
> 
>    lstat("file1", 0x7ffdedf9aca0)         = -1 ENOENT (No such file or directory)
>    mkdir("file1", 0777)                   = -1 EEXIST (File exists)
>    lstat("fifo1", {st_mode=S_IFIFO|0664, st_size=0, ...}) = 0
> 
> At that point the reason the mkdir actually failed was that it was a
> file but the second stat() would have a report that it was a fifo
> device which would be incorrect.  Race condition.
> 
> For the low level utilities it is almost always best to remain close
> to the kernel and report the actual error reported by the kernel.
> Constructing a layer in the low level utilities is almost always a bad
> idea.
> 
> However in your own shell scripts if you wish to construct exactly
> this same layer that you are proposing then that is your prerogative.
> But it would open your program up to valid bug reports for when the
> race condition tripped one of the problem cases.
> 
>> You're right, there will be a race condition where two processes are both
>> creating and deleting the same files. Any software which is creating and
>> deleting the same directories in parallel will encounter a multitude of
>> errors - all bets are off.
> 
> I am glad that you agree.  That is exactly why adding non-atomic
> operations to the low level utilities creating those race coditions is
> a bad idea.
> 
>>> A better fix would be to change the mkdir system call so that it sets errno to
>>> EISDIR in this situation. This would fix not only the mkdir utility, but also
>>> lots of other programs; and it wouldn't introduce a race condition. So if you're
>>> interested in getting the problem fixed, I suggest that you propose such a
>>> change to the Linux kernel developers.
>>
>> Yes, if Linux kernel developers would deviate from POSIX.  I emailed
>> linux-ext4 <at> vger.kernel.org the lines of code to change.
> 
> I am glad to see that you are following up in the right place.  That
> place being in the kernel.  It can't be addressed correctly in user space.
> 
>> I'm not confident it will get in, even harder to get into POSIX I expect.
>>
>> ext4_match() is what would need to be updated to check if an entry is a
>> directory
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/ext4/namei.c
> 
> ext4?  But what if I am using xfs?  Does this then behave differently
> on ext4 versus ext3 versus xfs versus nfs4 versus...  Well...  You get
> the idea.
> 
> Sometimes the only way to win is not to play.  Is the current EEXIST
> really so bad that we would create additional problems just to avoid
> it?  Simplest is almost always best.
> 
> Bob
> 

Thank you for your reply, and going through it all.

yes, I only asked Linux kernel ext4 team, as it is one place to ask the 
question, and judge the response. They also don't want to make any 
change. We're stuck with all these old interfaces. Unless someone wants 
to come up with mkdir2() and get it into POSIX.

Maybe a simple localised string is better in your mkdir tool? After-all 
the man page and POSIX gives the exact meaning of EEXIST in this context?

"pathname already exists"


The output is only incorrect because it defaults to system localised 
strerror(EEXIST)

So with this change, it would be:

mkdir: cannot create directory ‘test’: pathname already exists

Cheers, Jonny

Message #48 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Jonny Grant <jg <at> jguk.org>, Bob Proulx <bob <at> proulx.com>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Tue, 5 May 2020 14:58:02 -0500

On 5/5/20 1:36 PM, Jonny Grant wrote:

>>
>> Okay.  That's confusing.  The only value in hand being EEXIST then
>> that is the error to be reported.  If this were repeated many times
>> then sometimes we would catch it as an actual directory.
>>
>>    lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file or 
>> directory)
>>    mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
>>    lstat("foodir1", {st_mode=S_IFDIR|0775, st_size=40, ...}) = 0
>>
>> In that case the proposal is to report it as EISDIR.

> 
> yes, I only asked Linux kernel ext4 team, as it is one place to ask the 
> question, and judge the response. They also don't want to make any 
> change. We're stuck with all these old interfaces. Unless someone wants 
> to come up with mkdir2() and get it into POSIX.

We already have mkdirat() specified by POSIX.  It would be easier to add 
a new O_ flag that tells mkdirat() to give a different errno failure 
than to add a completely new interface.  But emulating that new flag on 
older kernels that don't natively support it will be back at the same 
non-atomic racy situation we are in now.

> 
> Maybe a simple localised string is better in your mkdir tool? After-all 
> the man page and POSIX gives the exact meaning of EEXIST in this context?
> 
> "pathname already exists"

If you can convince glibc to change the contents of their 
strerror(EEXIST) along those lines, then go for it.  But they would 
probably tell you that the GNU Coding Standards frown on using 
"pathname" instead of "filename" (per GCS, "path" should be reserved for 
colon-separated lists such as $PATH - which is a different definition 
than POSIX has where "path" is merely a concatenation of "filenames" 
using '/', so it's a tough sell.

> 
> 
> The output is only incorrect because it defaults to system localised 
> strerror(EEXIST)
> 
> So with this change, it would be:
> 
> mkdir: cannot create directory ‘test’: pathname already exists

Giving different output than strerror() will confuse users; it's better 
to make the change in glibc for ALL clients of strerror(EEXIST) rather 
than just this one client.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Message #51 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: taehwan jeoung <taehwanjeoung <at> yahoo.com>
To: 41001 <at> debbugs.gnu.org, Jonny Grant <jg <at> jguk.org>
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Wed, 6 May 2020 02:53:29 +0000 (UTC)

[Message part 1 (text/plain, inline)]

 Stupid.mkdir -r test.mkdir test.Read your gnu manual to solve your digital illiteracy.
    2020년 5월 2일 토요일 오전 12시 17분 45초 GMT+9, Jonny Grant <jg <at> jguk.org> 작성:  
 
 Hello!

Can this error message be clarified? The directory already exists, it is 
not a file.

lib/mkdir-p.c:200 contains this line of code that triggers below:-

error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));

As it's easy enough to know that the reason mkdir fails is because 
'test' a directory that already exists.

Easy enough to check with stat() and S_ISDIR(sb.st_mode)

Can this be changed? Maybe I can make a patch for it.
Jonny



$ mkdir test
$ mkdir test
mkdir: cannot create directory ‘test’: File exists
$ mkdir --version
mkdir (GNU coreutils) 8.28
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by David MacKenzie.
$



  

[Message part 2 (text/html, inline)]

Message #54 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Bob Proulx <bob <at> proulx.com>
To: taehwan jeoung <taehwanjeoung <at> yahoo.com>, Jonny Grant <jg <at> jguk.org>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Tue, 5 May 2020 22:34:52 -0600

taehwan jeoung wrote:
> Can this error message be clarified? The directory already exists, it is 
> not a file.

That is incorrect.  Directories are files.  FIFOs are files.  Device
nodes are files.  Symlinks are files.  Network sockets are files.
They are all files.  Therefore it is not incorrect to say that a file
already exists.  Directories are files.

We have all agreed that if a better error message were provided then
that would be an improvement.  We agree with you.  We would do it if
it were within the power of mkdir(1) to do it.  But it isn't.
Therefore we can't.

> lib/mkdir-p.c:200 contains this line of code that triggers below:-
> 
> error (0, mkdir_errno, _("cannot create directory %s"), quote (dir));
> 
> As it's easy enough to know that the reason mkdir fails is because 
> 'test' a directory that already exists.

That is also incorrect.  Since that information is not provided at the
time of the action it can only be inferred by implication later.  But
at the time of the failure return it cannot be known unless the kernel
provides that information.  Later in time things might have changed.

> Easy enough to check with stat() and S_ISDIR(sb.st_mode)

Incorrect.  Checking *later* with stat() does not provide the reason
that the earlier mkdir(2) failed.  It provides a guess of something
that might be the reason.  Maybe.  Or it maybe not.  Things may have
changed later in time and the guess made later might not be the
correct reason.  Reporting that as if it were would be a worse bug.
That checking later in time after the mkdir has failed is what
introduces the race condition that we have been talking about.  Please
do not ignore that critically important point.

> Can this be changed? Maybe I can make a patch for it.

Sigh.  Ignoring the reasons why this is a bad idea are not helpful.

Bob

Message #57 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Jonny Grant <jg <at> jguk.org>
To: Eric Blake <eblake <at> redhat.com>, Bob Proulx <bob <at> proulx.com>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Thu, 7 May 2020 12:29:58 +0100

Hi Eric

On 05/05/2020 20:58, Eric Blake wrote:
> On 5/5/20 1:36 PM, Jonny Grant wrote:
> 
>>>
>>> Okay.  That's confusing.  The only value in hand being EEXIST then
>>> that is the error to be reported.  If this were repeated many times
>>> then sometimes we would catch it as an actual directory.
>>>
>>>    lstat("foodir1", 0x7ffcafc12800)       = -1 ENOENT (No such file 
>>> or directory)
>>>    mkdir("foodir1", 0777)                 = -1 EEXIST (File exists)
>>>    lstat("foodir1", {st_mode=S_IFDIR|0775, st_size=40, ...}) = 0
>>>
>>> In that case the proposal is to report it as EISDIR.
> 
>>
>> yes, I only asked Linux kernel ext4 team, as it is one place to ask 
>> the question, and judge the response. They also don't want to make any 
>> change. We're stuck with all these old interfaces. Unless someone 
>> wants to come up with mkdir2() and get it into POSIX.
> 
> We already have mkdirat() specified by POSIX.  It would be easier to add 
> a new O_ flag that tells mkdirat() to give a different errno failure 
> than to add a completely new interface.  But emulating that new flag on 
> older kernels that don't natively support it will be back at the same 
> non-atomic racy situation we are in now.

Sounds good.

Would that O_ flag work on the 'mode' ? in which case could be mkdir() too.

>>
>> Maybe a simple localised string is better in your mkdir tool? 
>> After-all the man page and POSIX gives the exact meaning of EEXIST in 
>> this context?
>>
>> "pathname already exists"
> 
> If you can convince glibc to change the contents of their 
> strerror(EEXIST) along those lines, then go for it.  But they would 
> probably tell you that the GNU Coding Standards frown on using 
> "pathname" instead of "filename" (per GCS, "path" should be reserved for 
> colon-separated lists such as $PATH - which is a different definition 
> than POSIX has where "path" is merely a concatenation of "filenames" 
> using '/', so it's a tough sell.



"name already exists" might work, or "Directory or file already exists". 
Although it's almost certainly impossible, they also won't want to 
change it for strerror(EEXIST)

This comment is based on something Andreas Dilger suggested on the ext4 
list:

How about updating mkdir tool with this?

if (EEXIST == errno)
{
    errmsg = _("name already exists");
}


>> The output is only incorrect because it defaults to system localised 
>> strerror(EEXIST)
>>
>> So with this change, it would be:
>>
>> mkdir: cannot create directory ‘test’: pathname already exists
> 
> Giving different output than strerror() will confuse users; it's better 
> to make the change in glibc for ALL clients of strerror(EEXIST) rather 
> than just this one client.

I doubt glibc would ever agree to change strerror(EEXIST). I imagine it 
all gets beaurocratic, and they might require POSIX make the update to 
the spec first?


Cheers, Jonny

Message #60 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Jonny Grant <jg <at> jguk.org>, Bob Proulx <bob <at> proulx.com>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Thu, 7 May 2020 21:06:01 -0500

On 5/7/20 6:29 AM, Jonny Grant wrote:

>>
>> We already have mkdirat() specified by POSIX.  It would be easier to 
>> add a new O_ flag that tells mkdirat() to give a different errno 
>> failure than to add a completely new interface.  But emulating that 
>> new flag on older kernels that don't natively support it will be back 
>> at the same non-atomic racy situation we are in now.
> 
> Sounds good.
> 
> Would that O_ flag work on the 'mode' ? in which case could be mkdir() too.

Bummer. I forgot that mkdirat() was one of the interfaces that lacks a 
separate flags parameter (I was thinking more about openat).

But ultimately, whether the kernel adds a mkdirat4(dirfd, name, mode, 
flags), or merely adds flags that can be OR'd in with mode, is up to the 
kernel folks.  You'll have to convince someone in the kernel that an 
extension to the existing interface is worthwhile.  Ruminating on what 
that extension might look like here on the coreutils list won't get it 
any closer to actually being implemented.

(My personal wish: I would love a variation of mkdir that returns an 
open fd on the just-created directory on success in a single syscall, 
instead of the current practice of having to pair mkdir()/open() - 
something that is also doable if you have a flags parameter to opt-in to 
that new behavior.)

>> Giving different output than strerror() will confuse users; it's 
>> better to make the change in glibc for ALL clients of strerror(EEXIST) 
>> rather than just this one client.
> 
> I doubt glibc would ever agree to change strerror(EEXIST). I imagine it 
> all gets beaurocratic, and they might require POSIX make the update to 
> the spec first?

POSIX does _not_ require that strerror(EEXIST) output "File exists", but 
merely that it represent _some_ error message about a directory entry 
already existing (most generically called a file, even when it is not a 
regular file).  I see no reason why glibc developers would be able to 
use POSIX as a reason why they could not change their output table.  But 
you are also right that they might have other arguments about why 
changing an output string is undesirable.  But to find out, you'll have 
to ask on the glibc list, not here.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Message #63 received at 41001 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eric Blake <eblake <at> redhat.com>, Jonny Grant <jg <at> jguk.org>,
 Bob Proulx <bob <at> proulx.com>
Cc: 41001 <at> debbugs.gnu.org
Subject: Re: bug#41001: mkdir: cannot create directory ‘test’: File exists
Date: Sun, 10 May 2020 12:07:42 -0700

On 5/7/20 7:06 PM, Eric Blake wrote:
> 
> (My personal wish: I would love a variation of mkdir that returns an open fd on
> the just-created directory on success in a single syscall,

Yes! That would be a worthy addition.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.