/fs/ext4/namei.c ext4_find_dest_de()

/fs/ext4/namei.c ext4_find_dest_de()

[Jonny Grant]

Hi

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.

This is the line of code from ext4_find_dest_de():

if (ext4_match(dir, fname, de))
			return -EEXIST;



I propose to change to something like the following:


int ext4_match_result = ext4_match(dir, fname, de);

		nlen = EXT4_DIR_REC_LEN(de->name_len);
		rlen = ext4_rec_len_from_disk(de->rec_len, buf_size);
		if ((de->inode ? rlen - nlen : rlen) >= reclen)
			break;
		de = (struct ext4_dir_entry_2 *)((char *)de + rlen);

if (ext4_match_result)
{
     if(EXT4_FT_DIR == de->file_type)
     {
         return -EISDIR;
     }
     else
     {
	return -EEXIST;
     }
}



Let me know if this would be supported, and I can prepare a patch.

Cheers
Jonny

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

[Theodore Y. Ts'o]

On Sun, May 03, 2020 at 02:00:25PM +0100, Jonny Grant wrote:
> Hi
> 
> 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/

Regards,

						- Ted

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

[Jonny Grant]

On 04/05/2020 02:51, Theodore Y. Ts'o wrote:
> On Sun, May 03, 2020 at 02:00:25PM +0100, Jonny Grant wrote:
>> Hi
>>
>> 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/
> 
> Regards,
> 
> 						- Ted

Hi,

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.

As  developer, I can see it feels more confusing for users as it is. 
This issue shows up in various programs.

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


I would expect it to be clear for users:

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


The 'mkdir' team don't want to add a call to stat() to give a more 
appropriate error message.

Cheers, Jonny

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

[Theodore Y. Ts'o]

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.

Best regards,

						- Ted

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

[Jonny Grant]

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

Cheers, Jonny

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

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2811 bytes --]

On May 5, 2020, at 12:07 PM, Jonny Grant <jg@jguk.org> 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 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?

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

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

[Jonny Grant]

On 05/05/2020 19:50, Andreas Dilger wrote:
> On May 5, 2020, at 12:07 PM, Jonny Grant <jg@jguk.org> 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

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

[Jonny Grant]

On 04/05/2020 02:51, Theodore Y. Ts'o wrote:
> On Sun, May 03, 2020 at 02:00:25PM +0100, Jonny Grant wrote:
>> Hi
>>
>> 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/
> 
> Regards,
> 
> 						- Ted
> 

Thank you
This is the POSIX mkdir():
https://pubs.opengroup.org/onlinepubs/009695399/functions/mkdir.html

How about adding an improved mkdir to POSIX and the Linux kernel? Let's call it mkdir2()

It has one more error, for EISDIR

[EEXIST]
The named file exists.

[EISDIR]
Directory with that name exists.




I'm tempted to suggest this new function mkdir2() returns 0 on success, or an error number directly number. That would 
do away with 'errno' for this as well.

Regards, Jonny

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

[Theodore Y. Ts'o]

On Wed, May 27, 2020 at 10:25:43PM +0100, Jonny Grant wrote:
> 
> 
> How about adding an improved mkdir to POSIX and the Linux kernel? Let's call it mkdir2()
> 
> It has one more error, for EISDIR
> 
> [EEXIST]
> The named file exists.
> 
> [EISDIR]
> Directory with that name exists.

It's *really* not worth it.

You seem to really care about this; why don't you just keep your own
version of shellutils which calls stat(1) if you get EEXIST and to
distinguish between these two cases?

I know the shellutils maintainers has rejected this.  But that's OK,
you can have your own copy on your systems.  You might want to reflect
that if the shellutils maintainer refused to add the stat(1) on the
error path, what makes you think they will accept a change to use a
non-standard mkdir2() system call?

If you want to try to convince Austin Common Standards Revision Group
that it's worth it to mandate a whole new system call *just* for a new
error code, you're welcome to try.  I suspect you will not get a good
reception, and even if you did, Linux VFS maintainer may well very
well deride the proposal as silly, and refuse to follow the lead of
the Austin group.  In fact, Al Viro is very likely not to be as
politic as I have been.  (It's likely the response would have included
things like "idiotic idea" and "stupid".)

> I'm tempted to suggest this new function mkdir2() returns 0 on success, or
> an error number directly number. That would do away with 'errno' for this as
> well.

This is not going to get a good reception from either Al or the Austin
Group, I predict.  If you really have strong ideas of what you think
an OS and its interfaces should look like, perhaps you should just
design your own OS from scratch.

Best regards,

						- Ted

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

[Jonny Grant]

On 28/05/2020 02:12, Theodore Y. Ts'o wrote:
> On Wed, May 27, 2020 at 10:25:43PM +0100, Jonny Grant wrote:
>>
>>
>> How about adding an improved mkdir to POSIX and the Linux kernel? Let's call it mkdir2()
>>
>> It has one more error, for EISDIR
>>
>> [EEXIST]
>> The named file exists.
>>
>> [EISDIR]
>> Directory with that name exists.
> 
> It's *really* not worth it.

You're right.

> You seem to really care about this; why don't you just keep your own
> version of shellutils which calls stat(1) if you get EEXIST and to
> distinguish between these two cases?
> 
> I know the shellutils maintainers has rejected this.  But that's OK,
> you can have your own copy on your systems.  You might want to reflect
> that if the shellutils maintainer refused to add the stat(1) on the
> error path, what makes you think they will accept a change to use a
> non-standard mkdir2() system call?

You're right, it's unlikely.

> If you want to try to convince Austin Common Standards Revision Group
> that it's worth it to mandate a whole new system call *just* for a new
> error code, you're welcome to try.  I suspect you will not get a good
> reception, and even if you did, Linux VFS maintainer may well very
> well deride the proposal as silly, and refuse to follow the lead of
> the Austin group.  In fact, Al Viro is very likely not to be as
> politic as I have been.  (It's likely the response would have included
> things like "idiotic idea" and "stupid".)
> 
>> I'm tempted to suggest this new function mkdir2() returns 0 on success, or
>> an error number directly number. That would do away with 'errno' for this as
>> well.
> 
> This is not going to get a good reception from either Al or the Austin
> Group, I predict.  If you really have strong ideas of what you think
> an OS and its interfaces should look like, perhaps you should just
> design your own OS from scratch.

Yes, I agree, no one would want to change anything.
I recall other APIs like pthread_setname_np return 0 on success, and the error code directly, so there is some trend in 
that direction. That's part of POSIX.1


Hmm designing an OS, could be fun as a hobby, but wouldn't be big and professional like linux is on x86 x64 these days. 
I'd probably seek feedback on things people like/dislike in POSIX, as any OS would resemble it somewhat anyway, at least 
due to practical reasons compiling common tools. It would end up with a POSIX wrapper, on top of any APIs I designed, 
and then there would be no reason to use my APIs anyway. It's more fun to contribute to something with global appeal 
than a hobby project.

Regards, Jonny