On 02/17/2016 08:23 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>
>> The exit path for SCLD_EXISTS wasn't setting errno, as expected by at
>> least one caller. Fix the problem and document that the function sets
>> errno correctly to help avoid similar regressions in the future.
>
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 568120e..a1ac646 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -135,8 +135,10 @@ enum scld_error safe_create_leading_directories(char *path)
>> *slash = '\0';
>> if (!stat(path, &st)) {
>> /* path exists */
>> - if (!S_ISDIR(st.st_mode))
>> + if (!S_ISDIR(st.st_mode)) {
>> + errno = EEXIST;
>> ret = SCLD_EXISTS;
>
> Hmm, when does this trigger? There is a non-directory A/B, you are
> preparing leading directories to create A/B/C/D, and you find A/B
> exists but is not a directory so you cannot create A/B/C underneath
> it?
>
> That sounds more like ENOTDIR to me.
Yes, you're right. Thanks.
> Does the caller expect EEXIST, or both?
I don't know of any callers that branch based on errno, but several use
strerror() or die_errno() to report the error to the user. The few
callers that care about the reason for the failure base their behavior
on the return value.
But there are a lot of callers and I haven't audited each of them
carefully. Given that this error path currently doesn't necessarily set
errno *at all*, I think using a plausible value is strictly an improvement.
I'll change it to ENOTDIR in the next round.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html