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

[Junio C Hamano <gitster@pobox.com>]

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

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


>  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;
>  }