Re: [PATCH v2 2/2] grep: stop looking at random places for .gitattributes

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Oct 15, 2012, 06:02 UTC
Message-ID: <507BA6F0.4090500@viscovery.net>
In-Reply-To: <7v4nlx3cc8.fsf@alter.siamese.dyndns.org>

Am 10/14/2012 6:29, schrieb Junio C Hamano:

Show 17 quoted lines

> 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