Hi Hannes,

On Sat, 20 May 2017, Johannes Sixt wrote:

> Fetching from a remote using a native Windows path produces these warnings:
> 
> C:\Temp\gittest>git fetch C:\Temp\gittest
> warning: unable to access '.git/remotes/C:\Temp\gittest': Invalid argument
> warning: unable to access '.git/branches/C:\Temp\gittest': Invalid argument
> From C:\Temp\gittest
>  * branch            HEAD       -> FETCH_HEAD
> 
> The functions that read the branches and remotes files are protected by
> a valid_remote_nick() check. Its implementation does not take Windows
> paths into account, so that the caller simply concatenates the paths,
> leading to the error seen above.
> 
> Use is_dir_sep() to check for both slashes and backslashes on Windows.
> 
> Signed-off-by: Johannes Sixt <j...@kdbg.org>

I like this explanation.

> diff --git a/remote.c b/remote.c
> index 9584af366d..9501357c06 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -649,7 +649,10 @@ static int valid_remote_nick(const char *name)
>  {
>       if (!name[0] || is_dot_or_dotdot(name))
>               return 0;
> -     return !strchr(name, '/'); /* no slash */
> +     while (*name)
> +             if (is_dir_sep(*name++))        /* no slash */

I understand that you simply kept the comment, yet I have to admit that I
find it confusing in the post image. Something like

        /* remote nicknames cannot contain slashes */

(and putting it *above* the `if` line instead of appending it separated by
a <tab>) would have made it easier on me.

> +                     return 0;
> +     return 1;
>  }
>  

With my suggested change or without it, ACK.

Ciao,
Dscho

Reply via email to