Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit "slash" pointer

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2014, 18:32 UTC
Message-ID: <xmqqppo5hsf9.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1389015935-21936-4-git-send-email-mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 6 quoted lines

> Keep track of the position of the slash character independently of
> "pos", thereby making the purpose of each variable clearer and
> working towards other upcoming changes.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---

This step has an interaction with $gmane/239878 where Windows folks want it to pay attention to is_dir_sep()---over there, a backslash could separate directory path components.

AFAIK, the function was meant to be used only on paths we internally generate, and the paths we internally generate all are slash separated, so it could be argued that feeding a path, whose path components are separated by backslashes, that we obtained from the end user without converting it to the internal form in some codepaths (e.g. "$there" in "git clone $url $there") are bugs we acquired over time that need to be fixed, but it is easy enough to use is_dir_sep() here to work it around, and doing so will not negatively affect

 1. UNIX-only projects by forbidding use of a byte with backslash in
    it as a path component character (yes, I am imagining using
    Shift-JIS that can use a backslash as the second byte of
    two-byte character in the pathname on UNIX); and

 2. UNIX-and-Windows mixed projects, as you cannot sanely use such a
    pathname with backslash as part of a path component if its tree
    needs to be checked out on Windows.

Show 53 quoted lines

>  sha1_file.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index cc9957e..197766d 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path)
>  
>  	while (pos) {
>  		struct stat st;
> +		char *slash = strchr(pos, '/');
>  
> -		pos = strchr(pos, '/');
> -		if (!pos)
> +		if (!slash)
>  			break;
> -		while (*++pos == '/')
> -			;
> +		while (*(slash + 1) == '/')
> +			slash++;
> +		pos = slash + 1;
>  		if (!*pos)
>  			break;
> -		*--pos = '\0';
> +
> +		*slash = '\0';
>  		if (!stat(path, &st)) {
>  			/* path exists */
>  			if (!S_ISDIR(st.st_mode)) {
> -				*pos = '/';
> +				*slash = '/';
>  				return -3;
>  			}
>  		} else if (mkdir(path, 0777)) {
> @@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path)
>  			    !stat(path, &st) && S_ISDIR(st.st_mode)) {
>  				; /* somebody created it since we checked */
>  			} else {
> -				*pos = '/';
> +				*slash = '/';
>  				return -1;
>  			}
>  		} else if (adjust_shared_perm(path)) {
> -			*pos = '/';
> +			*slash = '/';
>  			return -2;
>  		}
> -		*pos++ = '/';
> +		*slash = '/';
>  	}
>  	return 0;
>  }