git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Johannes Sixt <j.sixt@viscovery.net>
Date
Oct 17, 2012, 07:05 UTC
Message-ID
<507E58CF.2040803@viscovery.net>
In-Reply-To
<507D010A.8000904@viscovery.net>
Am 10/16/2012 8:39, schrieb Johannes Sixt:
Show 15 quoted lines
> Am 10/15/2012 18:54, schrieb Junio C Hamano:
>> Ideally, that earlier workaround
>> should have done a logica equivalent of:
>> ...
>> and did so not in-line at the calling site but in a compat/ wrapper
>> for fflush() to eliminate the need for the ifdef.
> 
> Fair enough.
> 
>>> But reverting the EINVAL check from write_or_die.c is out of question,
>>> because that handles a real case.
> 
> Correction: I can't reproduce the error messages that this was working
> around anymore in a brief test. I'll revert the check locally and see what
> happens.
The error is reproducible with this command on Windows:

$ git rev-list HEAD | sed 1q 42b333d8bb8709dfc5783dd4c44bdb6012c2c17d fatal: write failure on 'stdout': Invalid argument

Let's do as you suggested.
--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] maybe_flush_or_die: move a too-loose Windows specific error
 check to compat

Commit b2f5e268 (Windows: Work around an oddity when a pipe with no reader is written to) introduced a check for EINVAL after fflush() to fight spurious "Invalid argument" errors on Windows when a pipe was broken. But this check may hide real errors on systems that do not have the this odd behavior. Introduce an fflush wrapper in compat/mingw.* so that the treatment is only applied on Windows.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 compat/mingw.c | 22 ++++++++++++++++++++++
 compat/mingw.h |  3 +++
 write_or_die.c |  7 +------
 3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index afc892d..4e63838 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -335,6 +335,28 @@ FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
 	return freopen(filename, otype, stream);
 }
 
+#undef fflush
+int mingw_fflush(FILE *stream)
+{
+	int ret = fflush(stream);
+
+	/*
+	 * write() is used behind the scenes of stdio output functions.
+	 * Since git code does not check for errors after each stdio write
+	 * operation, it can happen that write() is called by a later
+	 * stdio function even if an earlier write() call failed. In the
+	 * case of a pipe whose readable end was closed, only the first
+	 * call to write() reports EPIPE on Windows. Subsequent write()
+	 * calls report EINVAL. It is impossible to notice whether this
+	 * fflush invocation triggered such a case, therefore, we have to
+	 * catch all EINVAL errors whole-sale.
+	 */
+	if (ret && errno == EINVAL)
+		errno = EPIPE;
+
+	return ret;
+}
+
 /*
  * The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
  * Returns the 100-nanoseconds ("hekto nanoseconds") since the epoch.
diff --git a/compat/mingw.h b/compat/mingw.h
index 61a6521..eeb08d1 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -185,6 +185,9 @@ FILE *mingw_fopen (const char *filename, const char *otype);
 FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream);
 #define freopen mingw_freopen
 
+int mingw_fflush(FILE *stream);
+#define fflush mingw_fflush
+
 char *mingw_getcwd(char *pointer, int len);
 #define getcwd mingw_getcwd
 
diff --git a/write_or_die.c b/write_or_die.c
index d45b536..960f448 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -34,12 +34,7 @@ void maybe_flush_or_die(FILE *f, const char *desc)
 			return;
 	}
 	if (fflush(f)) {
-		/*
-		 * On Windows, EPIPE is returned only by the first write()
-		 * after the reading end has closed its handle; subsequent
-		 * write()s return EINVAL.
-		 */
-		if (errno == EPIPE || errno == EINVAL)
+		if (errno == EPIPE)
 			exit(0);
 		die_errno("write failure on '%s'", desc);
 	}
-- 
1.8.0.rc2.1248.g3f5b13f
Previous: Johannes SixtNext: Junio C Hamano
Message 38 of 43 in “'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree”
  1. Johannes SixtOct 9, 2012
  2. Nguyen Thai Ngoc DuyOct 9, 2012
  3. Nguyen Thai Ngoc DuyOct 9, 2012
  4. Jeff KingOct 9, 2012
  5. Junio C HamanoOct 9, 2012
  6. Nguyen Thai Ngoc DuyOct 10, 2012
  7. Junio C HamanoOct 10, 2012
  8. Nguyen Thai Ngoc DuyOct 10, 2012
  9. Nguyễn Thái Ngọc DuyOct 10, 2012
  10. 1/3 quote: let caller reset buffer for quote_path_relative()Nguyễn Thái Ngọc Duy, Oct 10, 2012
  11. Junio C HamanoOct 10, 2012
  12. Nguyen Thai Ngoc DuyOct 11, 2012
  13. Junio C HamanoOct 11, 2012
  14. 2/3 grep: pass true path name to grep machineryNguyễn Thái Ngọc Duy, Oct 10, 2012
  15. 3/3 grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 10, 2012
  16. Johannes SixtOct 10, 2012
  17. Nguyen Thai Ngoc DuyOct 10, 2012
  18. Johannes SixtOct 10, 2012
  19. Nguyen Thai Ngoc DuyOct 10, 2012
  20. Johannes SixtOct 10, 2012
  21. Nguyen Thai Ngoc DuyOct 10, 2012
  22. Junio C HamanoOct 10, 2012
  23. Johannes SixtOct 11, 2012
  24. Michael HaggertyOct 11, 2012
  25. Nguyen Thai Ngoc DuyOct 11, 2012
  26. 0/2 Re: 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktreeNguyễn Thái Ngọc Duy, Oct 10, 2012
  27. 1/2 quote: let caller reset buffer for quote_path_relative()Nguyễn Thái Ngọc Duy, Oct 10, 2012
  28. 2/2 grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 10, 2012
  29. Johannes SixtOct 10, 2012
  30. Junio C HamanoOct 10, 2012
  31. Johannes SixtOct 11, 2012
  32. Junio C HamanoOct 11, 2012
  33. Johannes SixtOct 12, 2012
  34. Junio C HamanoOct 14, 2012
  35. Johannes SixtOct 15, 2012
  36. Junio C HamanoOct 15, 2012
  37. Johannes SixtOct 16, 2012
  38. Johannes SixtOct 17, 2012
  39. Junio C HamanoOct 17, 2012
  40. Nguyen Thai Ngoc DuyOct 11, 2012
  41. Junio C HamanoOct 11, 2012
  42. grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 12, 2012
  43. Junio C HamanoOct 12, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.