Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jonny Grant <jg@jguk.org>
Subject: Re: /fs/ext4/namei.c ext4_find_dest_de()
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     "Theodore Y. Ts'o" <tytso@mit.edu>, linux-ext4@vger.kernel.org
References: <2edc7d6a-289e-57ad-baf1-477dc240474d@jguk.org>
 <20200504015122.GB404484@mit.edu>
 <b518357b-4c79-910a-94dc-b6f0125309bc@jguk.org>
 <20200504195255.GC404484@mit.edu>
 <f1d8d13f-1605-a19c-e75c-1ecdb8c42fcf@jguk.org>
 <1AB6A7B0-245F-4951-A2BC-E6EA1495D505@dilger.ca>
Message-ID: <636e24b3-b286-dc21-48b8-0097b8782a23@jguk.org>
Date:   Thu, 7 May 2020 12:25:10 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <1AB6A7B0-245F-4951-A2BC-E6EA1495D505@dilger.ca>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 8bit
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk


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