Re: [PATCH 02/20] safe_create_leading_directories(): set errno on SCLD_EXISTS

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 17, 2016, 19:23 UTC
Message-ID: <xmqqziuzi1jo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <7ddc8ac89f36b01494fbdc6f97bf1ca258b3e885.1455626201.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> 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.

Show 12 quoted lines

> 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.

Does the caller expect EEXIST, or both?

> +			}
>  		} else if (mkdir(path, 0777)) {
>  			if (errno == EEXIST &&
>  			    !stat(path, &st) && S_ISDIR(st.st_mode))