Subject: Re: [PATCH v3 2/2] Windows: do not treat a path with backslashes as a remote's nick name

From: Junio C Hamano <[email-masked]>
Date: Fri, 26 May 2017 08:16:09 +0900

Johannes Sixt <[email-masked]> writes:

>>> So in short:
>>>
>>>   (1) Hannes's patches are good, but they solve a problem that is
>>>       different from what their log messages say; the log message
>>>       needs to be updated;
>
> I do not resend patch 1/2 as it is unchanged in all regards. This 2/2
> changes the justification; patch text is unchanged.

Thanks.  I think this is explained better.  Complaints against
fopen() warnings sounded as if we should avoid attempting to open a
file that may result in _any_ failure, which I felt was misleading,
but it is not a huge issue.

So how do we want to proceed on the point (2), i.e. updating the
"warn on _unexpected_ errors from fopen" series to make it aware of
the EINVAL we can expect on Windows?  My primary question is if all
EINVAL we could ever see on Windows after open/fopen returns an
error is because the pathname the caller gave is not liked by the
filesystem (hence we also know that the path does not exist).

If that is the case, then the "workaround" patch I sent would be an
OK approach (even though I do not know what to write after #ifdef
and I suspect that is not "WINDOWS". We would want to cover the one
you use, the one Dscho releases and possibly the cygwin build).

If we can see EINVAL after open/fopen error that is _not_ expected
and indicates a failure that is worth reporting to the user (just
like we want to report e.g. I/O or permission errors), I think
Windows folks are in a better position than I am to decide between
that approach and a patch at lower level (e.g. teach open/fopen not
to give EINVAL and instead give ENOENT when appropriate).

>  remote.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index ad6c5424ed..1949882c10 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -645,7 +645,12 @@ static int valid_remote_nick(const char *name)
>  {
>  	if (!name[0] || is_dot_or_dotdot(name))
>  		return 0;
> -	return !strchr(name, '/'); /* no slash */
> +
> +	/* remote nicknames cannot contain slashes */
> +	while (*name)
> +		if (is_dir_sep(*name++))
> +			return 0;
> +	return 1;
>  }
>  
>  const char *remote_for_branch(struct branch *branch, int *explicit)