git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	stefan.naewe@atlas-elektronik.com, git@vger.kernel.org
Subject: Re: [PATCH v3 2/2] Windows: do not treat a path with backslashes as a remote's nick name
Date: Fri, 26 May 2017 08:16:09 +0900	[thread overview]
Message-ID: <xmqq37bseddy.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <10ee6b91-cd54-d43a-4cfb-d3baa2af7e7a@kdbg.org> (Johannes Sixt's message of "Thu, 25 May 2017 14:00:13 +0200")

Johannes Sixt <j6t@kdbg.org> 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)

  reply	other threads:[~2017-05-25 23:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-20  6:28 [PATCH 0/2] Fix warnings on access of a remote with Windows paths Johannes Sixt
2017-05-20  6:28 ` [PATCH 1/2] mingw.h: permit arguments with side effects for is_dir_sep Johannes Sixt
2017-05-22 11:52   ` Johannes Schindelin
2017-05-20  6:28 ` [PATCH 2/2] Windows: do not treat a path with backslashes as a remote's nick name Johannes Sixt
2017-05-22 11:56   ` Johannes Schindelin
2017-05-22 11:59 ` [PATCH 0/2] Fix warnings on access of a remote with Windows paths Johannes Schindelin
2017-05-22 16:36   ` Johannes Sixt
2017-05-23 10:53     ` Johannes Schindelin
2017-05-23 18:39       ` Johannes Sixt
2017-05-22 12:38 ` stefan.naewe
2017-05-22 14:01   ` Johannes Schindelin
2017-05-22 16:28     ` Johannes Sixt
2017-05-23 10:46       ` Johannes Schindelin
2017-05-23 22:08         ` Junio C Hamano
2017-05-24  5:45           ` Johannes Sixt
2017-05-25 12:00             ` [PATCH v3 2/2] Windows: do not treat a path with backslashes as a remote's nick name Johannes Sixt
2017-05-25 23:16               ` Junio C Hamano [this message]
2017-05-22 18:58 ` [PATCH v2 1/2] mingw.h: permit arguments with side effects for is_dir_sep Johannes Sixt
2017-05-22 19:01   ` [PATCH v2 2/2] Windows: do not treat a path with backslashes as a remote's nick name Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq37bseddy.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=stefan.naewe@atlas-elektronik.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).