Re: /fs/ext4/namei.c ext4_find_dest_de()

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

 





On 05/05/2020 19:50, Andreas Dilger wrote:
On May 5, 2020, at 12:07 PM, Jonny Grant <jg@xxxxxxxx> wrote:
On 04/05/2020 20:52, Theodore Y. Ts'o wrote:
On Mon, May 04, 2020 at 08:38:33AM +0100, Jonny Grant wrote:
I noticed that mkdir() returns EEXIST if a directory already exists.
strerror(EEXIST) text is "File exists"

Can ext4_find_dest_de() be amended to return EISDIR if a directory already
exists? This will make the error message clearer.

No; this will confuse potentially a large number of existing programs.
Also, the current behavior is required by POSIX and the Single Unix
Specification standards.

	https://pubs.opengroup.org/onlinepubs/009695399/

Is it likely POSIX would introduce this change? It's a shame we're still
constrained by old standards (SVr4, BSD), but it's fine if they can be
updated.
No, because it has the potential to break existing Unix/Linux/Posix-compliant
programs.  There may very well be C programs doing the following....
	   if (mkdir(filename) < 0) {
	   	if (errno != EEXIST) {
			perror(filename);
			exit(1);
		}
	}
For example, there may very well be implementations of "mkdir -p" that
do precisely this.
If we change the error returned by the mkdir system call as you
propose, it would break these innocent, unsuspecting programs.  That's
not something which will be allowed, because it falls into the
category of a Bad Thing.

Thank you for your reply.

What's an appropriate solution to this problem?

To achieve the desired output. when a directory exists.

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

I don't think it is reasonable to change the EEXIST return code just
to make you happy.  However, it may be within your purview to change
the mkdir(1) code to improve the error message:

	rc = mkdir(name, mode);
	if (rc < 0) {
		if (errno == EEXIST)
			errmsg = _("File or directory already exists");
                 else
                         errmsg = strerror(errno);
		fprintf(stderr, "%s: cannot create directory '%s': %s\n",
			progname, name, errmsg);
         }

or whatever you want.  If you are really keen, you could try to change
the string that strerror(EEXIST) provides to be more generic, but that
may also break programs that parse the output of mkdir(1) for some reason.

I doubt glibc would ever agree to change strerror(EEXIST).


I would *not* recommend to change this to stat() the target name to
determine the file type just to print the error message, as that is just
useless overhead, of which there is too much in GNU fileutils already.

On the flip side, what is the driver for making this change?  The existing
error message has been OK for users for 40 years already?

It's a pet peeve, saw it in some logs from our software, wondered if the message could be clear as I knew there is the EISDIR errno.

I recall someone else mentioned adding another mode flag, O_ to allow the kernel to return EISDIR if it knows that, that would work, then programs could enable it in the future.

Cheers, Jonny



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux