Thread (28 messages) flat view 28 messages, 9 authors, 2016-06-15

Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 22:58:26

On 2013-08-17 14.40, Steffen Prohaska wrote:
quoted hunk ↗ jump to hunk
Previously, filtering more than 2GB through an external filter (see
test) failed on Mac OS X 10.8.4 (12E55) with:

    error: read from external filter cat failed
    error: cannot feed the input to external filter cat
    error: cat died of signal 13
    error: external filter cat failed 141
    error: external filter cat failed

The reason is that read() immediately returns with EINVAL if len >= 2GB.
I haven't found any information under which specific conditions this
occurs.  My suspicion is that it happens when reading from a pipe, while
reading from a standard file should always be fine.  I haven't tested
any other version of Mac OS X, though I'd expect that other versions are
affected as well.

The problem is fixed by always reading less than 2GB in xread().
xread() doesn't guarantee to read all the requested data at once, and
callers are expected to gracefully handle partial reads.  Slicing large
reads into 2GB pieces should not hurt practical performance.

Signed-off-by: Steffen Prohaska <redacted>
---
 t/t0021-conversion.sh | 9 +++++++++
 wrapper.c             | 8 ++++++++
 2 files changed, 17 insertions(+)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e50f0f7..aec1253 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '
 	test_must_fail git add test.fc
 '
 
+test_expect_success 'filter large file' '
+	git config filter.largefile.smudge cat &&
+	git config filter.largefile.clean cat &&
+	dd if=/dev/zero of=2GB count=2097152 bs=1024 &&
+	echo "/2GB filter=largefile" >.gitattributes &&
+	git add 2GB 2>err &&
+	! grep -q "error" err
+'
+
 test_done
diff --git a/wrapper.c b/wrapper.c
index 6a015de..2a2f496 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)
 {
 	ssize_t nr;
 	while (1) {
+#ifdef __APPLE__
+		const size_t twoGB = (1l << 31);
+		/* len >= 2GB immediately fails on Mac OS X with EINVAL when
+		 * reading from pipe. */
+		if (len >= twoGB) {
+			len = twoGB - 1;
+		}
+#endif
 		nr = read(fd, buf, len);
 		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
 			continue;
Thanks for the patch.
I think we can we can replace  __APPLE__ define with a more generic one.
We had a similar patch for write() some time ago:

config.mak.uname
	NEEDS_CLIPPED_WRITE = YesPlease


Makefile
ifdef NEEDS_CLIPPED_WRITE
	BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
	COMPAT_OBJS += compat/clipped-write.o
endif
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help