All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	git@vger.kernel.org, "Jeff King" <peff@peff.net>
Subject: Re: [PATCH v2 2/2] grep: stop looking at random places for .gitattributes
Date: Mon, 15 Oct 2012 08:02:24 +0200	[thread overview]
Message-ID: <507BA6F0.4090500@viscovery.net> (raw)
In-Reply-To: <7v4nlx3cc8.fsf@alter.siamese.dyndns.org>

Am 10/14/2012 6:29, schrieb Junio C Hamano:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> It might be worth it. We already have a similar special case in
>> write_or_die.c:maybe_flush_or_die() for Windows, although it is not about
>> a colon in a path name.
>>
>> Perhaps like this.
> 
> Hrm, the "we already have" one b2f5e26 (Windows: Work around an
> oddity when a pipe with no reader is written to., 2007-08-17) was
> what you added while I was looking the other way ;-) as a part of
> Windows specific pull.
> 
> That change, and this patch, seem to cover the cases to be ignored
> with a bit too wide a net to my taste.  On other systems, and even
> on Windows if the path does not have any colon, EINVAL is something
> we would want to noticbbe and report, as a potential problem, no?

For fopen(), EINVAL should occur only if the mode argument is wrong, which
it isn't. For fflush() (as in write_or_die.c), EINVAL is not even listed
as possible error code. Therefore, catching EINVAL wholesale should not be
a problem, IMO, at least not "on other systems".

On Windows, it is more problematic because there is a table of "customary"
Windows API error codes, which are mapped to errno values, and EINVAL is
used for all other Windows error codes (and for a few listed ones), and
ignoring EINVAL might indeed miss something worth to be reported.

Sooo... I don't mind if you do not pick up this patch because it handles a
rather theoretic case, i.e., where a project with strange paths somehow
ended up on a Windows drive.

But reverting the EINVAL check from write_or_die.c is out of question,
because that handles a real case.

-- Hannes

  reply	other threads:[~2012-10-15  6:02 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-09  9:03 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree Johannes Sixt
2012-10-09  9:38 ` Nguyen Thai Ngoc Duy
2012-10-09 12:01   ` Nguyen Thai Ngoc Duy
2012-10-09 12:41     ` Jeff King
2012-10-09 18:59       ` Junio C Hamano
2012-10-10  5:17         ` Nguyen Thai Ngoc Duy
2012-10-10  5:33           ` Junio C Hamano
2012-10-10  5:45             ` Nguyen Thai Ngoc Duy
2012-10-10 11:34         ` Nguyễn Thái Ngọc Duy
2012-10-10 11:34           ` [PATCH 1/3] quote: let caller reset buffer for quote_path_relative() Nguyễn Thái Ngọc Duy
2012-10-10 21:13             ` Junio C Hamano
2012-10-11 13:04               ` Nguyen Thai Ngoc Duy
2012-10-11 16:42                 ` Junio C Hamano
2012-10-10 11:34           ` [PATCH 2/3] grep: pass true path name to grep machinery Nguyễn Thái Ngọc Duy
2012-10-10 11:34           ` [PATCH 3/3] grep: stop looking at random places for .gitattributes Nguyễn Thái Ngọc Duy
2012-10-10 11:51             ` Johannes Sixt
2012-10-10 12:03               ` Nguyen Thai Ngoc Duy
2012-10-10 12:12                 ` Johannes Sixt
2012-10-10 12:32                   ` Nguyen Thai Ngoc Duy
2012-10-10 12:43                     ` Johannes Sixt
2012-10-10 12:51                       ` Nguyen Thai Ngoc Duy
2012-10-10 19:44                   ` Junio C Hamano
2012-10-11  5:55                     ` Johannes Sixt
2012-10-11  7:04                       ` Michael Haggerty
2012-10-11  8:17                         ` Nguyen Thai Ngoc Duy
2012-10-10 13:59           ` [PATCH v2 0/2] Re: 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree Nguyễn Thái Ngọc Duy
2012-10-10 13:59             ` [PATCH v2 1/2] quote: let caller reset buffer for quote_path_relative() Nguyễn Thái Ngọc Duy
2012-10-10 13:59             ` [PATCH v2 2/2] grep: stop looking at random places for .gitattributes Nguyễn Thái Ngọc Duy
2012-10-10 14:21               ` Johannes Sixt
2012-10-10 19:56                 ` Junio C Hamano
2012-10-11  5:45                   ` Johannes Sixt
2012-10-11 15:51                     ` Junio C Hamano
2012-10-12  7:33                       ` Johannes Sixt
2012-10-14  4:29                         ` Junio C Hamano
2012-10-15  6:02                           ` Johannes Sixt [this message]
2012-10-15 16:54                             ` Junio C Hamano
2012-10-16  6:39                               ` Johannes Sixt
2012-10-17  7:05                                 ` Johannes Sixt
2012-10-17  7:33                                   ` Junio C Hamano
2012-10-11  1:49                 ` Nguyen Thai Ngoc Duy
2012-10-11  3:15                   ` Junio C Hamano
2012-10-12 10:49               ` [PATCH v3] " Nguyễn Thái Ngọc Duy
2012-10-12 16:47                 ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=507BA6F0.4090500@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.