{"thread":{"id":"34717","subject":"[PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","startedAt":"2013-08-17T12:40:05Z","lastAt":"2013-08-27T04:59:14Z","messageCount":37,"participants":["Steffen Prohaska","John Keeping","Torsten Bögershausen","Johannes Sixt","Jonathan Nieder","Kyle J. McKay","Stefan Beller","Eric Sunshine","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"225369","messageId":"1376743205-12618-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":null,"subject":"[PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-17T12:40:05Z","receivedAt":"2013-08-17T12:40:05Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering more than 2GB through an external filter (see\ntest) failed on Mac OS X 10.8.4 (12E55) with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason is that read() immediately returns with EINVAL if len >= 2GB.\nI haven't found any information under which specific conditions this\noccurs. My suspicion is that it happens when reading from a pipe, while\nreading from a standard file should always be fine. I haven't tested\nany other version of Mac OS X, though I'd expect that other versions are\naffected as well.\n\nThe problem is fixed by always reading less than 2GB in xread().\nxread() doesn't guarantee to read all the requested data at once, and\ncallers are expected to gracefully handle partial reads. Slicing large\nreads into 2GB pieces should not hurt practical performance.\n\nSigned-off-by: Steffen Prohaska \n---\n t/t0021-conversion.sh | 9 +++++++++\n wrapper.c | 8 ++++++++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..aec1253 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test_expect_success 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tdd if=/dev/zero of=2GB count=2097152 bs=1024 &&\n+\techo \"/2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! grep -q \"error\" err\n+'\n+\n test_done\ndiff --git a/wrapper.c b/wrapper.c\nindex 6a015de..2a2f496 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)\n {\n \tssize_t nr;\n \twhile (1) {\n+#ifdef __APPLE__\n+\t\tconst size_t twoGB = (1l << 31);\n+\t\t/* len >= 2GB immediately fails on Mac OS X with EINVAL when\n+\t\t * reading from pipe. */\n+\t\tif (len >= twoGB) {\n+\t\t\tlen = twoGB - 1;\n+\t\t}\n+#endif\n \t\tnr = read(fd, buf, len);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n \t\t\tcontinue;\n-- \n1.8.4.rc3.5.gfcb973a\n"},{"id":"225373","messageId":"20130817152729.GR2337@serenity.lan","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-08-17T15:27:29Z","receivedAt":"2013-08-17T15:27:29Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sat, Aug 17, 2013 at 02:40:05PM +0200, Steffen Prohaska wrote:\n> Previously, filtering more than 2GB through an external filter (see\n> test) failed on Mac OS X 10.8.4 (12E55) with:\n> \n> error: read from external filter cat failed\n> error: cannot feed the input to external filter cat\n> error: cat died of signal 13\n> error: external filter cat failed 141\n> error: external filter cat failed\n> \n> The reason is that read() immediately returns with EINVAL if len >= 2GB.\n> I haven't found any information under which specific conditions this\n> occurs. My suspicion is that it happens when reading from a pipe, while\n> reading from a standard file should always be fine. I haven't tested\n> any other version of Mac OS X, though I'd expect that other versions are\n> affected as well.\n> \n> The problem is fixed by always reading less than 2GB in xread().\n> xread() doesn't guarantee to read all the requested data at once, and\n> callers are expected to gracefully handle partial reads. Slicing large\n> reads into 2GB pieces should not hurt practical performance.\n> \n> Signed-off-by: Steffen Prohaska \n> ---\n> t/t0021-conversion.sh | 9 +++++++++\n> wrapper.c | 8 ++++++++\n> 2 files changed, 17 insertions(+)\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index e50f0f7..aec1253 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '\n> \ttest_must_fail git add test.fc\n> '\n> \n> +test_expect_success 'filter large file' '\n> +\tgit config filter.largefile.smudge cat &&\n> +\tgit config filter.largefile.clean cat &&\n> +\tdd if=/dev/zero of=2GB count=2097152 bs=1024 &&\n> +\techo \"/2GB filter=largefile\" >.gitattributes &&\n> +\tgit add 2GB 2>err &&\n> +\t! grep -q \"error\" err\n> +'\n> +\n> test_done\n> diff --git a/wrapper.c b/wrapper.c\n> index 6a015de..2a2f496 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)\n> {\n> \tssize_t nr;\n> \twhile (1) {\n> +#ifdef __APPLE__\n> +\t\tconst size_t twoGB = (1l << 31);\n> +\t\t/* len >= 2GB immediately fails on Mac OS X with EINVAL when\n> +\t\t * reading from pipe. */\n> +\t\tif (len >= twoGB) {\n> +\t\t\tlen = twoGB - 1;\n> +\t\t}\n\nPlease don't use unnecessary curly braces here (see\nDocumentation/CodingGuidelines).\n\n> +#endif\n> \t\tnr = read(fd, buf, len);\n> \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n> \t\t\tcontinue;\n> -- \n> 1.8.4.rc3.5.gfcb973a\n"},{"id":"225375","messageId":"520F9D20.7000008@web.de","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-08-17T15:56:16Z","receivedAt":"2013-08-17T15:56:16Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-08-17 14.40, Steffen Prohaska wrote:\n> Previously, filtering more than 2GB through an external filter (see\n> test) failed on Mac OS X 10.8.4 (12E55) with:\n> \n> error: read from external filter cat failed\n> error: cannot feed the input to external filter cat\n> error: cat died of signal 13\n> error: external filter cat failed 141\n> error: external filter cat failed\n> \n> The reason is that read() immediately returns with EINVAL if len >= 2GB.\n> I haven't found any information under which specific conditions this\n> occurs. My suspicion is that it happens when reading from a pipe, while\n> reading from a standard file should always be fine. I haven't tested\n> any other version of Mac OS X, though I'd expect that other versions are\n> affected as well.\n> \n> The problem is fixed by always reading less than 2GB in xread().\n> xread() doesn't guarantee to read all the requested data at once, and\n> callers are expected to gracefully handle partial reads. Slicing large\n> reads into 2GB pieces should not hurt practical performance.\n> \n> Signed-off-by: Steffen Prohaska \n> ---\n> t/t0021-conversion.sh | 9 +++++++++\n> wrapper.c | 8 ++++++++\n> 2 files changed, 17 insertions(+)\n> \n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index e50f0f7..aec1253 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '\n> \ttest_must_fail git add test.fc\n> '\n> \n> +test_expect_success 'filter large file' '\n> +\tgit config filter.largefile.smudge cat &&\n> +\tgit config filter.largefile.clean cat &&\n> +\tdd if=/dev/zero of=2GB count=2097152 bs=1024 &&\n> +\techo \"/2GB filter=largefile\" >.gitattributes &&\n> +\tgit add 2GB 2>err &&\n> +\t! grep -q \"error\" err\n> +'\n> +\n> test_done\n> diff --git a/wrapper.c b/wrapper.c\n> index 6a015de..2a2f496 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)\n> {\n> \tssize_t nr;\n> \twhile (1) {\n> +#ifdef __APPLE__\n> +\t\tconst size_t twoGB = (1l << 31);\n> +\t\t/* len >= 2GB immediately fails on Mac OS X with EINVAL when\n> +\t\t * reading from pipe. */\n> +\t\tif (len >= twoGB) {\n> +\t\t\tlen = twoGB - 1;\n> +\t\t}\n> +#endif\n> \t\tnr = read(fd, buf, len);\n> \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n> \t\t\tcontinue;\n> \nThanks for the patch.\nI think we can we can replace __APPLE__ define with a more generic one.\nWe had a similar patch for write() some time ago:\n\nconfig.mak.uname\n\tNEEDS_CLIPPED_WRITE = YesPlease\n\n\nMakefile\nifdef NEEDS_CLIPPED_WRITE\n\tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n\tCOMPAT_OBJS += compat/clipped-write.o\nendif\n"},{"id":"225377","messageId":"520FB000.7020000@kdbg.org","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-08-17T17:16:48Z","receivedAt":"2013-08-17T17:16:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.08.2013 14:40, schrieb Steffen Prohaska:\n> Previously, filtering more than 2GB through an external filter (see\n> test) failed on Mac OS X 10.8.4 (12E55) with:\n>\n> error: read from external filter cat failed\n> error: cannot feed the input to external filter cat\n> error: cat died of signal 13\n> error: external filter cat failed 141\n> error: external filter cat failed\n>\n> The reason is that read() immediately returns with EINVAL if len >= 2GB.\n> I haven't found any information under which specific conditions this\n> occurs. My suspicion is that it happens when reading from a pipe, while\n> reading from a standard file should always be fine. I haven't tested\n> any other version of Mac OS X, though I'd expect that other versions are\n> affected as well.\n>\n> The problem is fixed by always reading less than 2GB in xread().\n> xread() doesn't guarantee to read all the requested data at once, and\n> callers are expected to gracefully handle partial reads. Slicing large\n> reads into 2GB pieces should not hurt practical performance.\n>\n> Signed-off-by: Steffen Prohaska \n> ---\n> t/t0021-conversion.sh | 9 +++++++++\n> wrapper.c | 8 ++++++++\n> 2 files changed, 17 insertions(+)\n>\n> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\n> index e50f0f7..aec1253 100755\n> --- a/t/t0021-conversion.sh\n> +++ b/t/t0021-conversion.sh\n> @@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '\n> \ttest_must_fail git add test.fc\n> '\n>\n> +test_expect_success 'filter large file' '\n> +\tgit config filter.largefile.smudge cat &&\n> +\tgit config filter.largefile.clean cat &&\n> +\tdd if=/dev/zero of=2GB count=2097152 bs=1024 &&\n\nWe don't have /dev/zero on Windows. Even if we get a file slightly over \n2GB, we can't handle it on Windows, and other 32bit architectures will \nvery likely also be handicapped.\n\nFinally, this test (if it remains in some form) should probably be \nprotected by EXPENSIVE.\n\n> +\techo \"/2GB filter=largefile\" >.gitattributes &&\n\nDrop the slash, please; it may confuse our bash on Windows (it doesn't \ncurrently because echo is a builtin, but better safe than sorry).\n\n> +\tgit add 2GB 2>err &&\n> +\t! grep -q \"error\" err\n\nExecutive summary: drop everything starting at \"2>err\".\n\nLong story: Can it happen that (1) git add succeeds, but still produces \nsomething on stderr, and (2) we do not care what this something is as long \nas it does not contain \"error\"? I don't think this combination of \nconditions makes sense; it's sufficient to check that git add does not fail.\n\nBTW, if you add\n\n\t... &&\n\trm -f 2GB &&\n\tgit checkout -- 2GB\n\nyou would also test the smudge filter code path with a huge file, no?\n\nBTW2, to create a file with slightly over 2GB, you can use\n\n\tfor i in $(test_seq 0 128); do printf \"%16777216d\" 1; done >2GB\n\n> +'\n> +\n> test_done\n> diff --git a/wrapper.c b/wrapper.c\n> index 6a015de..2a2f496 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)\n> {\n> \tssize_t nr;\n> \twhile (1) {\n> +#ifdef __APPLE__\n> +\t\tconst size_t twoGB = (1l << 31);\n> +\t\t/* len >= 2GB immediately fails on Mac OS X with EINVAL when\n> +\t\t * reading from pipe. */\n> +\t\tif (len >= twoGB) {\n> +\t\t\tlen = twoGB - 1;\n> +\t\t}\n> +#endif\n> \t\tnr = read(fd, buf, len);\n> \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n> \t\t\tcontinue;\n>\n\n-- Hannes\n"},{"id":"225381","messageId":"20130817185759.GA2904@elie.Belkin","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-08-17T18:57:59Z","receivedAt":"2013-08-17T18:57:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nSteffen Prohaska wrote:\n\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)\n> {\n> \tssize_t nr;\n> \twhile (1) {\n> +#ifdef __APPLE__\n> +\t\tconst size_t twoGB = (1l << 31);\n> +\t\t/* len >= 2GB immediately fails on Mac OS X with EINVAL when\n> +\t\t * reading from pipe. */\n> +\t\tif (len >= twoGB) {\n> +\t\t\tlen = twoGB - 1;\n> +\t\t}\n> +#endif\n> \t\tnr = read(fd, buf, len);\n\nSee 6c642a87 (compat: large write(2) fails on Mac OS X/XNU,\n2013-05-10) for a cleaner way to do this.\n\nHope that helps,\nJonathan\n"},{"id":"225383","messageId":"0EC822B9-E5BB-4FF5-B054-167866EA2075@gmail.com","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-08-17T20:25:45Z","receivedAt":"2013-08-17T20:25:45Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Aug 17, 2013, at 05:40, Steffen Prohaska wrote:\n\n> Previously, filtering more than 2GB through an external filter (see\n> test) failed on Mac OS X 10.8.4 (12E55) with:\n>\n> error: read from external filter cat failed\n> error: cannot feed the input to external filter cat\n> error: cat died of signal 13\n> error: external filter cat failed 141\n> error: external filter cat failed\n>\n> The reason is that read() immediately returns with EINVAL if len >= \n> 2GB.\n> I haven't found any information under which specific conditions this\n> occurs.\n\nAccording to POSIX [1] for read:\n\nIf the value of nbyte is greater than {SSIZE_MAX}, the result is \nimplementation-defined.\n\n\nThe write function also has the same restriction [2].\n\nSince OS X still supports running 32-bit executables, and SSIZE_MAX is \n2GB - 1 when running 32-bit it would seem the same limit has been \nimposed on 64-bit executables. In any case, we should avoid \n\"implementation-defined\" behavior for portability unless we know the \nOS we were compiled on has acceptable \"implementation-defined\" \nbehavior and otherwise never attempt to read or write more than \nSSIZE_MAX bytes.\n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n"},{"id":"225385","messageId":"20130817212314.GC2904@elie.Belkin","threadId":"34717","inReplyTo":"0EC822B9-E5BB-4FF5-B054-167866EA2075@gmail.com","subject":"Re: [PATCH] xread(): Fix read error when filtering >= 2GB on Mac OS X","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-08-17T21:23:14Z","receivedAt":"2013-08-17T21:23:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kyle J. McKay wrote:\n\n> According to POSIX [1] for read:\n>\n> If the value of nbyte is greater than {SSIZE_MAX}, the result is\n> implementation-defined.\n\nSure.\n\n[...]\n> Since OS X still supports running 32-bit executables, and SSIZE_MAX is 2GB -\n> 1 when running 32-bit it would seem the same limit has been imposed on\n> 64-bit executables. In any case, we should avoid \"implementation-defined\"\n> behavior\n\nWait --- that's a big leap.\n\nIn a 64-bit executable, SSIZE_MAX is 2^63 - 1, so the behavior is not\nimplementation-defined. I'm not sure if Steffen's copy of git is\n32-bit or 64-bit --- my guess would be 64-bit. So at first glance\nthis does look like an XNU-specific bug, not a standards thing.\n\nWhat about the related case where someone does try to \"git add\"\na file with a clean filter producing more than SSIZE_MAX and less\nthan SIZE_MAX bytes?\n\nstrbuf_grow() does not directly protect against a strbuf growing to >\nSSIZE_MAX bytes, but in practice on most machines realloc() does. So\nin practice we could never read more than SSIZE_MAX characters in the\nstrbuf_read() codepath, but it might be worth a check for paranoia\nanyway.\n\nWhile we're here, it's easy to wonder: why is git reading into such a\nlarge buffer anyway? Normally git uses the streaming codepath for\nfiles larger than big_file_threshold (typically 512 MiB).\nUnfortunately there are cases where it doesn't. For example:\n\n - convert_to_git() has not been taught to stream, so paths\n with a clean filter or requiring crlf conversion are read or\n mapped into memory.\n\n - deflate_to_pack() relies on seeking backward to retry when\n a pack would grow too large, so \"git hash-object --stdin\"\n cannot use that codepath.\n\n - a \"clean\" filter can make a file bigger.\n\n Perhaps git needs to learn to write to a temporary file\n when asked to keep track of a blob that is larger than fits\n reasonably in memory. Or maybe not.\n\nSo there is room for related work but the codepaths that read()\nindefinitely large files do seem to be needed, at least in the short\nterm. Working around this Mac OS X-specific limitation at the read()\nlevel like you've done still sounds like the right thing to do.\n\nThanks, both, for your work tracking this down. Hopefully the next\nversion of the patch will be in good shape and then it can be applied\nquickly.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"225443","messageId":"1376894300-28929-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376743205-12618-1-git-send-email-prohaska@zib.de","subject":"[PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T06:38:20Z","receivedAt":"2013-08-19T06:38:20Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering 2GB or more through an external filter (see test)\nfailed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason was that read() immediately returns with EINVAL if nbyte >=\n2GB. According to POSIX [1], if the value of nbyte passed to read() is\ngreater than SSIZE_MAX, the result is implementation-defined. The write\nfunction has the same restriction [2]. Since OS X still supports\nrunning 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX\n= 2GB - 1) seems to be also imposed on 64-bit executables under certain\nconditions. For write, the problem has been addressed in a earlier\ncommit [6c642a].\n\nThe problem for read() is addressed in a similar way by introducing\na wrapper function in compat that always reads less than 2GB.\nUnfortunately, '#undef read' is needed at a few places to avoid\nexpanding the compat macro in constructs like 'vtbl->read(...)'.\n\nNote that 'git add' exits with 0 even if it prints filtering errors to\nstderr. The test, therefore, checks stderr. 'git add' should probably\nbe changed (sometime in another commit) to exit with nonzero if\nfiltering fails. The test could then be changed to use test_must_fail.\n\nThanks to the following people for their suggestions:\n\n Johannes Sixt \n John Keeping \n Jonathan Nieder \n Kyle J. McKay \n Torsten Bögershausen \n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n\n[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3\n compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\n\nSigned-off-by: Steffen Prohaska \n---\n Makefile | 8 ++++++++\n builtin/var.c | 1 +\n config.mak.uname | 1 +\n git-compat-util.h | 5 +++++\n streaming.c | 1 +\n t/t0021-conversion.sh | 14 ++++++++++++++\n 6 files changed, 30 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..0f69e24 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,6 +69,9 @@ all::\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n+# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than\n+# INT_MAX bytes at once (e.g. MacOS X).\n+#\n # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n # INT_MAX bytes at once (e.g. MacOS X).\n #\n@@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n+ifdef NEEDS_CLIPPED_READ\n+\tBASIC_CFLAGS += -DNEEDS_CLIPPED_READ\n+\tCOMPAT_OBJS += compat/clipped-read.o\n+endif\n+\n ifdef NEEDS_CLIPPED_WRITE\n \tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n \tCOMPAT_OBJS += compat/clipped-write.o\ndiff --git a/builtin/var.c b/builtin/var.c\nindex aedbb53..e59f5ba 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -38,6 +38,7 @@ static struct git_var git_vars[] = {\n \t{ \"\", NULL },\n };\n \n+#undef read\n static void list_vars(void)\n {\n \tstruct git_var *ptr;\ndiff --git a/config.mak.uname b/config.mak.uname\nindex b27f51d..5c10726 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin)\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n+\tNEEDS_CLIPPED_READ = YesPlease\n \tNEEDS_CLIPPED_WRITE = YesPlease\n \tCOMPAT_OBJS += compat/precompose_utf8.o\n \tBASIC_CFLAGS += -DPRECOMPOSE_UNICODE\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 115cb1d..a227127 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -185,6 +185,11 @@ typedef unsigned long uintptr_t;\n #define probe_utf8_pathname_composition(a,b)\n #endif\n \n+#ifdef NEEDS_CLIPPED_READ\n+ssize_t clipped_read(int fd, void *buf, size_t nbyte);\n+#define read(x,y,z) clipped_read((x),(y),(z))\n+#endif\n+\n #ifdef NEEDS_CLIPPED_WRITE\n ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);\n #define write(x,y,z) clipped_write((x),(y),(z))\ndiff --git a/streaming.c b/streaming.c\nindex debe904..c1fe34a 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)\n \treturn r;\n }\n \n+#undef read\n ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n {\n \treturn st->vtbl->read(st, buf, sz);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\n test_done\n-- \n1.8.4.rc3.5.ga3343dd\n"},{"id":"225444","messageId":"20130819075408.GT2337@serenity.lan","threadId":"34717","inReplyTo":"1376894300-28929-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-08-19T07:54:08Z","receivedAt":"2013-08-19T07:54:08Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Aug 19, 2013 at 08:38:20AM +0200, Steffen Prohaska wrote:\n> diff --git a/Makefile b/Makefile\n> index 3588ca1..0f69e24 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -69,6 +69,9 @@ all::\n> # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n> # doesn't support GNU extensions like --check and --statistics\n> #\n> +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than\n> +# INT_MAX bytes at once (e.g. MacOS X).\n> +#\n> # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n> # INT_MAX bytes at once (e.g. MacOS X).\n> #\n> @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n> \tMSGFMT += --check --statistics\n> endif\n> \n> +ifdef NEEDS_CLIPPED_READ\n> +\tBASIC_CFLAGS += -DNEEDS_CLIPPED_READ\n> +\tCOMPAT_OBJS += compat/clipped-read.o\n\nYou've created compat/clipped-write.c, but...\n\n> Makefile | 8 ++++++++\n> builtin/var.c | 1 +\n> config.mak.uname | 1 +\n> git-compat-util.h | 5 +++++\n> streaming.c | 1 +\n> t/t0021-conversion.sh | 14 ++++++++++++++\n> 6 files changed, 30 insertions(+)\n\n... it's not included here. Did you forget to \"git add\"?\n"},{"id":"225447","messageId":"5211D544.8080706@kdbg.org","threadId":"34717","inReplyTo":"1376894300-28929-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-08-19T08:20:20Z","receivedAt":"2013-08-19T08:20:20Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.08.2013 08:38, schrieb Steffen Prohaska:\n> +test_expect_success EXPENSIVE 'filter large file' '\n> +\tgit config filter.largefile.smudge cat &&\n> +\tgit config filter.largefile.clean cat &&\n> +\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n\nShouldn't you count to 2049 to get a file that is over 2GB?\n\n> +\techo \"2GB filter=largefile\" >.gitattributes &&\n> +\tgit add 2GB 2>err &&\n> +\t! test -s err &&\n> +\trm -f 2GB &&\n> +\tgit checkout -- 2GB 2>err &&\n> +\t! test -s err\n> +'\n\n-- Hannes\n"},{"id":"225448","messageId":"21189B7B-5413-447C-955B-784F51110B13@zib.de","threadId":"34717","inReplyTo":"20130819075408.GT2337@serenity.lan","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T08:20:59Z","receivedAt":"2013-08-19T08:20:59Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 19, 2013, at 9:54 AM, John Keeping wrote:\n\n> You've created compat/clipped-read.c, but...\n> \n>> Makefile | 8 ++++++++\n>> builtin/var.c | 1 +\n>> config.mak.uname | 1 +\n>> git-compat-util.h | 5 +++++\n>> streaming.c | 1 +\n>> t/t0021-conversion.sh | 14 ++++++++++++++\n>> 6 files changed, 30 insertions(+)\n> \n> ... it's not included here. Did you forget to \"git add\"?\n\nIndeed. How embarrassing. Thanks for spotting this. I'll send v3 in a minute.\n\n\tStefffen"},{"id":"225450","messageId":"1376900499-662-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376894300-28929-1-git-send-email-prohaska@zib.de","subject":"[PATCH v3] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T08:21:39Z","receivedAt":"2013-08-19T08:21:39Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering 2GB or more through an external filter (see test)\nfailed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason was that read() immediately returns with EINVAL if nbyte >=\n2GB. According to POSIX [1], if the value of nbyte passed to read() is\ngreater than SSIZE_MAX, the result is implementation-defined. The write\nfunction has the same restriction [2]. Since OS X still supports\nrunning 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX\n= 2GB - 1) seems to be also imposed on 64-bit executables under certain\nconditions. For write, the problem has been addressed in a earlier\ncommit [6c642a].\n\nThe problem for read() is addressed in a similar way by introducing\na wrapper function in compat that always reads less than 2GB.\nUnfortunately, '#undef read' is needed at a few places to avoid\nexpanding the compat macro in constructs like 'vtbl->read(...)'.\n\nNote that 'git add' exits with 0 even if it prints filtering errors to\nstderr. The test, therefore, checks stderr. 'git add' should probably\nbe changed (sometime in another commit) to exit with nonzero if\nfiltering fails. The test could then be changed to use test_must_fail.\n\nThanks to the following people for their suggestions:\n\n Johannes Sixt \n John Keeping \n Jonathan Nieder \n Kyle J. McKay \n Torsten Bögershausen \n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n\n[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3\n compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\n\nSigned-off-by: Steffen Prohaska \n---\n Makefile | 8 ++++++++\n builtin/var.c | 1 +\n compat/clipped-read.c | 13 +++++++++++++\n config.mak.uname | 1 +\n git-compat-util.h | 5 +++++\n streaming.c | 1 +\n t/t0021-conversion.sh | 14 ++++++++++++++\n 7 files changed, 43 insertions(+)\n create mode 100644 compat/clipped-read.c\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..0f69e24 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,6 +69,9 @@ all::\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n+# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than\n+# INT_MAX bytes at once (e.g. MacOS X).\n+#\n # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n # INT_MAX bytes at once (e.g. MacOS X).\n #\n@@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n+ifdef NEEDS_CLIPPED_READ\n+\tBASIC_CFLAGS += -DNEEDS_CLIPPED_READ\n+\tCOMPAT_OBJS += compat/clipped-read.o\n+endif\n+\n ifdef NEEDS_CLIPPED_WRITE\n \tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n \tCOMPAT_OBJS += compat/clipped-write.o\ndiff --git a/builtin/var.c b/builtin/var.c\nindex aedbb53..e59f5ba 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -38,6 +38,7 @@ static struct git_var git_vars[] = {\n \t{ \"\", NULL },\n };\n \n+#undef read\n static void list_vars(void)\n {\n \tstruct git_var *ptr;\ndiff --git a/compat/clipped-read.c b/compat/clipped-read.c\nnew file mode 100644\nindex 0000000..6962f67\n--- /dev/null\n+++ b/compat/clipped-read.c\n@@ -0,0 +1,13 @@\n+#include \"../git-compat-util.h\"\n+#undef read\n+\n+/*\n+ * Version of read that will write at most INT_MAX bytes.\n+ * Workaround a xnu bug on Mac OS X\n+ */\n+ssize_t clipped_read(int fd, void *buf, size_t nbyte)\n+{\n+\tif (nbyte > INT_MAX)\n+\t\tnbyte = INT_MAX;\n+\treturn read(fd, buf, nbyte);\n+}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex b27f51d..5c10726 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin)\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n+\tNEEDS_CLIPPED_READ = YesPlease\n \tNEEDS_CLIPPED_WRITE = YesPlease\n \tCOMPAT_OBJS += compat/precompose_utf8.o\n \tBASIC_CFLAGS += -DPRECOMPOSE_UNICODE\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 115cb1d..a227127 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -185,6 +185,11 @@ typedef unsigned long uintptr_t;\n #define probe_utf8_pathname_composition(a,b)\n #endif\n \n+#ifdef NEEDS_CLIPPED_READ\n+ssize_t clipped_read(int fd, void *buf, size_t nbyte);\n+#define read(x,y,z) clipped_read((x),(y),(z))\n+#endif\n+\n #ifdef NEEDS_CLIPPED_WRITE\n ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);\n #define write(x,y,z) clipped_write((x),(y),(z))\ndiff --git a/streaming.c b/streaming.c\nindex debe904..c1fe34a 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)\n \treturn r;\n }\n \n+#undef read\n ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n {\n \treturn st->vtbl->read(st, buf, sz);\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\n test_done\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225451","messageId":"5211D697.7020107@googlemail.com","threadId":"34717","inReplyTo":"5211D544.8080706@kdbg.org","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-08-19T08:25:59Z","receivedAt":"2013-08-19T08:25:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 08/19/2013 10:20 AM, Johannes Sixt wrote:\n> Am 19.08.2013 08:38, schrieb Steffen Prohaska:\n>> +test_expect_success EXPENSIVE 'filter large file' '\n>> + git config filter.largefile.smudge cat &&\n>> + git config filter.largefile.clean cat &&\n>> + for i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n> \n> Shouldn't you count to 2049 to get a file that is over 2GB?\n\nWould it be possible to offload the looping from shell to a real\nprogram? So for example\n\ttruncate -s 2049M \nshould do the job. That would create a file reading all bytes as zeros\t\nbeing larger as 2G. If truncate is not available, what about dd?\n\nStefan\n\n"},{"id":"225452","messageId":"5211D6DC.5090704@kdbg.org","threadId":"34717","inReplyTo":"1376894300-28929-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-08-19T08:27:08Z","receivedAt":"2013-08-19T08:27:08Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.08.2013 08:38, schrieb Steffen Prohaska:\n> Note that 'git add' exits with 0 even if it prints filtering errors to\n> stderr. The test, therefore, checks stderr. 'git add' should probably\n> be changed (sometime in another commit) to exit with nonzero if\n> filtering fails. The test could then be changed to use test_must_fail.\n\nThanks for this hint. I was not aware of this behavior.\n\nOf course, we do *not* want to use test_must_fail because git add \ngenerally must not fail for files with more than 2GB. (Architectures with \na 32bit size_t are a different matter, of course.)\n\n-- Hannes\n"},{"id":"225453","messageId":"B58CB9DC-029F-4568-8A27-0C60FFA6766B@zib.de","threadId":"34717","inReplyTo":"5211D544.8080706@kdbg.org","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T08:28:02Z","receivedAt":"2013-08-19T08:28:02Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 19, 2013, at 10:20 AM, Johannes Sixt wrote:\n\n> Am 19.08.2013 08:38, schrieb Steffen Prohaska:\n>> +test_expect_success EXPENSIVE 'filter large file' '\n>> +\tgit config filter.largefile.smudge cat &&\n>> +\tgit config filter.largefile.clean cat &&\n>> +\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n> \n> Shouldn't you count to 2049 to get a file that is over 2GB?\n\nNo. INT_MAX = 2GB - 1 works. INT_MAX + 1 = 2GB fails. It tests exactly at the boundary.\n\n\tSteffen\n"},{"id":"225455","messageId":"5211DA14.3020501@kdbg.org","threadId":"34717","inReplyTo":"5211D697.7020107@googlemail.com","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-08-19T08:40:52Z","receivedAt":"2013-08-19T08:40:52Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.08.2013 10:25, schrieb Stefan Beller:\n> On 08/19/2013 10:20 AM, Johannes Sixt wrote:\n>> Am 19.08.2013 08:38, schrieb Steffen Prohaska:\n>>> +test_expect_success EXPENSIVE 'filter large file' '\n>>> + git config filter.largefile.smudge cat &&\n>>> + git config filter.largefile.clean cat &&\n>>> + for i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n>>\n>> Shouldn't you count to 2049 to get a file that is over 2GB?\n>\n> Would it be possible to offload the looping from shell to a real\n> program? So for example\n> \ttruncate -s 2049M \n> should do the job. That would create a file reading all bytes as zeros\t\n> being larger as 2G. If truncate is not available, what about dd?\n\nThe point is exactly to avoid external dependencies. Our dd on Windows \ndoesn't do the right thing with seek=2GB (it makes the file twice as large \nas expected).\n\n-- Hannes\n"},{"id":"225456","messageId":"CAPig+cTr_B+vtN4sFzepWeW4TpRPD9eKnjy08yJ2pf3KfVU1XA@mail.gmail.com","threadId":"34717","inReplyTo":"1376900499-662-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v3] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-19T13:59:57Z","receivedAt":"2013-08-19T13:59:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Aug 19, 2013 at 4:21 AM, Steffen Prohaska wrote:\n> Previously, filtering 2GB or more through an external filter (see test)\n> failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n>\n> error: read from external filter cat failed\n> error: cannot feed the input to external filter cat\n> error: cat died of signal 13\n> error: external filter cat failed 141\n> error: external filter cat failed\n>\n>\n> Signed-off-by: Steffen Prohaska \n> ---\n> Makefile | 8 ++++++++\n> builtin/var.c | 1 +\n> compat/clipped-read.c | 13 +++++++++++++\n> config.mak.uname | 1 +\n> git-compat-util.h | 5 +++++\n> streaming.c | 1 +\n> t/t0021-conversion.sh | 14 ++++++++++++++\n> 7 files changed, 43 insertions(+)\n> create mode 100644 compat/clipped-read.c\n>\n> diff --git a/Makefile b/Makefile\n> index 3588ca1..0f69e24 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -69,6 +69,9 @@ all::\n> # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n> # doesn't support GNU extensions like --check and --statistics\n> #\n> +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than\n> +# INT_MAX bytes at once (e.g. MacOS X).\n> +#\n> # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n> # INT_MAX bytes at once (e.g. MacOS X).\n\nIs it likely that we would see a platform requiring only one or the\nother CLIPPED? Would it make sense to combine these into a single\nNEEDS_CLIPPED_IO?\n\n> #\n> @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n> MSGFMT += --check --statistics\n> endif\n>\n> +ifdef NEEDS_CLIPPED_READ\n> + BASIC_CFLAGS += -DNEEDS_CLIPPED_READ\n> + COMPAT_OBJS += compat/clipped-read.o\n> +endif\n> +\n> ifdef NEEDS_CLIPPED_WRITE\n> BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n> COMPAT_OBJS += compat/clipped-write.o\n> diff --git a/builtin/var.c b/builtin/var.c\n> index aedbb53..e59f5ba 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -38,6 +38,7 @@ static struct git_var git_vars[] = {\n> { \"\", NULL },\n> };\n"},{"id":"225458","messageId":"52122E8D.7030209@web.de","threadId":"34717","inReplyTo":"1376894300-28929-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-08-19T14:41:17Z","receivedAt":"2013-08-19T14:41:17Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-08-19 08.38, Steffen Prohaska wrote:\n[snip]\n\n> diff --git a/builtin/var.c b/builtin/var.c\n> index aedbb53..e59f5ba 100644\n> --- a/builtin/var.c\n> +++ b/builtin/var.c\n> @@ -38,6 +38,7 @@ static struct git_var git_vars[] = {\n> \t{ \"\", NULL },\n> };\n> \n> +#undef read\nThis is techically right for this very version of the code,\nbut not really future proof, if someone uses read() further down in the code\n(in a later version)\n\nI think the problem comes from further up:\n------------------\nstruct git_var {\n\tconst char *name;\n\tconst char *(*read)(int);\n};\n-----------------\ncould the read be replaced by readfn ?\n\n===================\n> diff --git a/streaming.c b/streaming.c\n> index debe904..c1fe34a 100644\n> --- a/streaming.c\n> +++ b/streaming.c\n> @@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)\n> \treturn r;\n> }\n> \n> +#undef read\nSame possible future problem as above.\nWhen later someone uses read, the original (buggy) read() will be\nused, and not the re-defined clipped_read() from git-compat-util.h\n"},{"id":"225459","messageId":"1376926879-30846-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376900499-662-1-git-send-email-prohaska@zib.de","subject":"[PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T15:41:19Z","receivedAt":"2013-08-19T15:41:19Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering 2GB or more through an external filter (see test)\nfailed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason was that read() immediately returns with EINVAL if nbyte >=\n2GB. According to POSIX [1], if the value of nbyte passed to read() is\ngreater than SSIZE_MAX, the result is implementation-defined. The write\nfunction has the same restriction [2]. Since OS X still supports\nrunning 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX\n= 2GB - 1) seems to be also imposed on 64-bit executables under certain\nconditions. For write, the problem has been addressed in a earlier\ncommit [6c642a].\n\nThe problem for read() is addressed in a similar way by introducing\na wrapper function in compat that always reads less than 2GB. It is\nvery likely that the read() and write() wrappers are always used\ntogether. To avoid introducing another option, NEEDS_CLIPPED_WRITE is\nchanged to NEEDS_CLIPPED_IO and used to activate both wrappers.\n\nTo avoid expanding the read compat macro in constructs like\n'vtbl->read(...)', 'read' is renamed to 'readfn' in two cases. The\nsolution seems more robust than using '#undef read'.\n\nNote that 'git add' exits with 0 even if it prints filtering errors to\nstderr. The test, therefore, checks stderr. 'git add' should probably\nbe changed (sometime in another commit) to exit with nonzero if\nfiltering fails. The test could then be changed to use test_must_fail.\n\nThanks to the following people for their suggestions:\n\n Johannes Sixt \n John Keeping \n Jonathan Nieder \n Kyle J. McKay \n Torsten Bögershausen \n Eric Sunshine \n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n\n[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3\n compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\n\nSigned-off-by: Steffen Prohaska \n---\n Makefile | 10 +++++-----\n builtin/var.c | 10 +++++-----\n compat/{clipped-write.c => clipped-io.c} | 11 ++++++++++-\n config.mak.uname | 2 +-\n git-compat-util.h | 5 ++++-\n streaming.c | 4 ++--\n t/t0021-conversion.sh | 14 ++++++++++++++\n 7 files changed, 41 insertions(+), 15 deletions(-)\n rename compat/{clipped-write.c => clipped-io.c} (53%)\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..f54134d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,8 +69,8 @@ all::\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n-# Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n-# INT_MAX bytes at once (e.g. MacOS X).\n+# Define NEEDS_CLIPPED_IO if your read(2) and/or write(2) cannot handle more\n+# than INT_MAX bytes at once (e.g. Mac OS X).\n #\n # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH\n # it specifies.\n@@ -1493,9 +1493,9 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n-ifdef NEEDS_CLIPPED_WRITE\n-\tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n-\tCOMPAT_OBJS += compat/clipped-write.o\n+ifdef NEEDS_CLIPPED_IO\n+\tBASIC_CFLAGS += -DNEEDS_CLIPPED_IO\n+\tCOMPAT_OBJS += compat/clipped-io.o\n endif\n \n ifneq (,$(XDL_FAST_HASH))\ndiff --git a/builtin/var.c b/builtin/var.c\nindex aedbb53..06f8459 100644\n--- a/builtin/var.c\n+++ b/builtin/var.c\n@@ -28,7 +28,7 @@ static const char *pager(int flag)\n \n struct git_var {\n \tconst char *name;\n-\tconst char *(*read)(int);\n+\tconst char *(*readfn)(int);\n };\n static struct git_var git_vars[] = {\n \t{ \"GIT_COMMITTER_IDENT\", git_committer_info },\n@@ -43,8 +43,8 @@ static void list_vars(void)\n \tstruct git_var *ptr;\n \tconst char *val;\n \n-\tfor (ptr = git_vars; ptr->read; ptr++)\n-\t\tif ((val = ptr->read(0)))\n+\tfor (ptr = git_vars; ptr->readfn; ptr++)\n+\t\tif ((val = ptr->readfn(0)))\n \t\t\tprintf(\"%s=%s\\n\", ptr->name, val);\n }\n \n@@ -53,9 +53,9 @@ static const char *read_var(const char *var)\n \tstruct git_var *ptr;\n \tconst char *val;\n \tval = NULL;\n-\tfor (ptr = git_vars; ptr->read; ptr++) {\n+\tfor (ptr = git_vars; ptr->readfn; ptr++) {\n \t\tif (strcmp(var, ptr->name) == 0) {\n-\t\t\tval = ptr->read(IDENT_STRICT);\n+\t\t\tval = ptr->readfn(IDENT_STRICT);\n \t\t\tbreak;\n \t\t}\n \t}\ndiff --git a/compat/clipped-write.c b/compat/clipped-io.c\nsimilarity index 53%\nrename from compat/clipped-write.c\nrename to compat/clipped-io.c\nindex b8f98ff..ec3232a 100644\n--- a/compat/clipped-write.c\n+++ b/compat/clipped-io.c\n@@ -1,10 +1,19 @@\n #include \"../git-compat-util.h\"\n+#undef read\n #undef write\n \n /*\n- * Version of write that will write at most INT_MAX bytes.\n+ * Versions of read() and write() that limit nbyte to INT_MAX.\n * Workaround a xnu bug on Mac OS X\n */\n+\n+ssize_t clipped_read(int fd, void *buf, size_t nbyte)\n+{\n+\tif (nbyte > INT_MAX)\n+\t\tnbyte = INT_MAX;\n+\treturn read(fd, buf, nbyte);\n+}\n+\n ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)\n {\n \tif (nbyte > INT_MAX)\ndiff --git a/config.mak.uname b/config.mak.uname\nindex b27f51d..fb39726 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -95,7 +95,7 @@ ifeq ($(uname_S),Darwin)\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n-\tNEEDS_CLIPPED_WRITE = YesPlease\n+\tNEEDS_CLIPPED_IO = YesPlease\n \tCOMPAT_OBJS += compat/precompose_utf8.o\n \tBASIC_CFLAGS += -DPRECOMPOSE_UNICODE\n endif\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 115cb1d..4a875cb 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -185,7 +185,10 @@ typedef unsigned long uintptr_t;\n #define probe_utf8_pathname_composition(a,b)\n #endif\n \n-#ifdef NEEDS_CLIPPED_WRITE\n+#ifdef NEEDS_CLIPPED_IO\n+ssize_t clipped_read(int fd, void *buf, size_t nbyte);\n+#define read(x,y,z) clipped_read((x),(y),(z))\n+\n ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);\n #define write(x,y,z) clipped_write((x),(y),(z))\n #endif\ndiff --git a/streaming.c b/streaming.c\nindex debe904..2ac3047 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -20,7 +20,7 @@ typedef ssize_t (*read_istream_fn)(struct git_istream *, char *, size_t);\n \n struct stream_vtbl {\n \tclose_istream_fn close;\n-\tread_istream_fn read;\n+\tread_istream_fn readfn;\n };\n \n #define open_method_decl(name) \\\n@@ -101,7 +101,7 @@ int close_istream(struct git_istream *st)\n \n ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)\n {\n-\treturn st->vtbl->read(st, buf, sz);\n+\treturn st->vtbl->readfn(st, buf, sz);\n }\n \n static enum input_source istream_source(const unsigned char *sha1,\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\n test_done\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225460","messageId":"CA+55aFzQhJqE4QDwJDKtkTtJpMNbz3_Aw5_Q3yTk5DnhLJyjCQ@mail.gmail.com","threadId":"34717","inReplyTo":"1376926879-30846-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-08-19T16:04:42Z","receivedAt":"2013-08-19T16:04:42Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Aug 19, 2013 at 8:41 AM, Steffen Prohaska wrote:\n>\n> The reason was that read() immediately returns with EINVAL if nbyte >=\n> 2GB. According to POSIX [1], if the value of nbyte passed to read() is\n> greater than SSIZE_MAX, the result is implementation-defined.\n\nYeah, the OS X filesystem layer is an incredible piece of shit. Not\nonly doesn't it follow POSIX, it fails *badly*. Because OS X kernel\nengineers apparently have the mental capacity of a retarded rodent on\ncrack.\n\nLinux also refuses to actually read more than a maximum value in one\ngo (because quite frankly, doing more than 2GB at a time is just not\nreasonable, especially in unkillable disk wait), but at least Linux\ngives you the partial read, so that the usual \"read until you're\nhappy\" works (which you have to do anyway with sockets, pipes, NFS\nintr mounts, etc etc). Returning EINVAL is a sign of a diseased mind.\n\nI hate your patch for other reasons, though:\n\n> The problem for read() is addressed in a similar way by introducing\n> a wrapper function in compat that always reads less than 2GB.\n\nWhy do you do that? We already _have_ wrapper functions for read(),\nnamely xread(). Exactly because you basically have to, in order to\nhandle signals on interruptible filesystems (which aren't POSIX\neither, but at least sanely so) or from other random sources. And to\nhandle the \"you can't do reads that big\" issue.\n\nSo why isn't the patch much more straightforward? Like the attached\ntotally untested one that just limits the read/write size to 8MB\n(which is totally arbitrary, but small enough to not have any latency\nissues even on slow disks, and big enough that any reasonable IO\nsubsystem will still get good throughput).\n\nAnd by \"totally untested\" I mean that it actually passes the git test\nsuite, but since I didn't apply your patch nor do I have OS X\nanywhere, I can't actually test that it fixes *your* problem. But it\nshould.\n\n\n Linus\n\n\n wrapper.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 6a015de5f056..e996f3dae467 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -131,6 +131,13 @@ void *xcalloc(size_t nmemb, size_t size)\n }\n \n /*\n+ * Doing IO in huge chunks only results in pain. OS X is buggy,\n+ * and even in the absense of bugs it can result in bad latencies\n+ * when you decide to kill the process.\n+ */\n+#define MAX_IO_SIZE (8*1024*1024)\n+\n+/*\n * xread() is the same a read(), but it automatically restarts read()\n * operations with a recoverable error (EAGAIN and EINTR). xread()\n * DOES NOT GUARANTEE that \"len\" bytes is read even if the data is available.\n@@ -139,7 +146,8 @@ ssize_t xread(int fd, void *buf, size_t len)\n {\n \tssize_t nr;\n \twhile (1) {\n-\t\tnr = read(fd, buf, len);\n+\t\tnr = len < MAX_IO_SIZE ? len : MAX_IO_SIZE;\n+\t\tnr = read(fd, buf, nr);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n \t\t\tcontinue;\n \t\treturn nr;\n@@ -155,7 +163,8 @@ ssize_t xwrite(int fd, const void *buf, size_t len)\n {\n \tssize_t nr;\n \twhile (1) {\n-\t\tnr = write(fd, buf, len);\n+\t\tnr = len < MAX_IO_SIZE ? len : MAX_IO_SIZE;\n+\t\tnr = write(fd, buf, nr);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n \t\t\tcontinue;\n \t\treturn nr;\n"},{"id":"225462","messageId":"7veh9pmyin.fsf@alter.siamese.dyndns.org","threadId":"34717","inReplyTo":"CAPig+cTr_B+vtN4sFzepWeW4TpRPD9eKnjy08yJ2pf3KfVU1XA@mail.gmail.com","subject":"Re: [PATCH v3] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-19T16:33:04Z","receivedAt":"2013-08-19T16:33:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than\n>> +# INT_MAX bytes at once (e.g. MacOS X).\n>> +#\n>> # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n>> # INT_MAX bytes at once (e.g. MacOS X).\n>\n> Is it likely that we would see a platform requiring only one or the\n> other CLIPPED? Would it make sense to combine these into a single\n> NEEDS_CLIPPED_IO?\n\nI am slightly negative to that suggestion for two reasons.\n\n - Does MacOS X clip other IO operations? Do we need to invent yet\n another NEEDS_CLIPPED, e.g. NEEDS_CLIPPED_LSEEK?\n\n A single NEEDS_CLIPPED_IO may sound attractive for its simplicity\n (e.g. on a system that only needs NEEDS_CLIPPED_WRITE, we will\n unnecessarily chop a big read into multiple reads, but that does\n not affect the correctness of the operation, only performance but\n the actual IO cost will dominate it anyway). If we know there\n are 47 different IO operations that might need clipping, that\n simplicity is certainly a good thing to have. I somehow do not\n think the set of operations will grow that large, though.\n\n - NEEDS_CLIPPED_IO essentially says \"only those who clip their\n writes would clip their reads (and vice versa)\", which is not all\n that different from saying \"only Apple would clip their IO\",\n which in turn defeats the notion of \"let's use a generic\n NEEDS_CLIPPED without limiting the workaround to specific\n platforms\" somewhat.\n"},{"id":"225463","messageId":"61B1EB04-497F-4398-8C52-CCE3A1A81B10@zib.de","threadId":"34717","inReplyTo":"CA+55aFzQhJqE4QDwJDKtkTtJpMNbz3_Aw5_Q3yTk5DnhLJyjCQ@mail.gmail.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-19T16:37:56Z","receivedAt":"2013-08-19T16:37:56Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Aug 19, 2013, at 6:04 PM, Linus Torvalds wrote:\n\n> I hate your patch for other reasons, though:\n> \n>> The problem for read() is addressed in a similar way by introducing\n>> a wrapper function in compat that always reads less than 2GB.\n> \n> Why do you do that? We already _have_ wrapper functions for read(),\n> namely xread(). Exactly because you basically have to, in order to\n> handle signals on interruptible filesystems (which aren't POSIX\n> either, but at least sanely so) or from other random sources. And to\n> handle the \"you can't do reads that big\" issue.\n> \n> So why isn't the patch much more straightforward? \n\nThe first version was more straightforward [1]. But reviewers suggested\nthat the compat wrappers would be the right way to do it and showed me\nthat it has been done like this before [2].\n\nI haven't submitted anything in a while, so I tried to be a kind person\nand followed the suggestions. I started to hate the patch a bit (maybe less\nthan you), but I wasn't brave enough to reject the suggestions. This is\nwhy the patch became what it is.\n\nI'm happy to rework it again towards your suggestion. I would also remove\nthe compat wrapper for write(). But I got a bit tired. I'd appreciate if\nI received more indication whether a version without compat wrappers would\nbe accepted.\n\n\tSteffen\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/232455\n[2] 6c642a8 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU"},{"id":"225466","messageId":"xmqqeh9p8ut3.fsf@gitster.dls.corp.google.com","threadId":"34717","inReplyTo":"CA+55aFzQhJqE4QDwJDKtkTtJpMNbz3_Aw5_Q3yTk5DnhLJyjCQ@mail.gmail.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-19T17:16:56Z","receivedAt":"2013-08-19T17:16:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I hate your patch for other reasons, though:\n>\n>> The problem for read() is addressed in a similar way by introducing\n>> a wrapper function in compat that always reads less than 2GB.\n>\n> Why do you do that? We already _have_ wrapper functions for read(),\n> namely xread(). Exactly because you basically have to, in order to\n> handle signals on interruptible filesystems (which aren't POSIX\n> either, but at least sanely so) or from other random sources. And to\n> handle the \"you can't do reads that big\" issue.\n\nThe same argument applies to xwrite(), but currently we explicitly\ncatch EINTR and EAGAIN knowing that on sane systems these are the\nsigns that we got interrupted.\n\nDo we catch EINVAL unconditionally in the same codepath? Could\nEINVAL on saner systems mean completely different thing (like our\ncaller is passing bogus parameters to underlying read/write, which\nis a program bug we would want to catch)?\n\n> So why isn't the patch much more straightforward? Like the attached\n> totally untested one that just limits the read/write size to 8MB\n> (which is totally arbitrary, but small enough to not have any latency\n> issues even on slow disks, and big enough that any reasonable IO\n> subsystem will still get good throughput).\n\nAhh. OK, not noticing EINVAL unconditionally, but always feed IOs\nin chunks that are big enough for sane systems but small enough for\nbroken ones.\n\nThat makes sense. Could somebody on MacOS X test this?\n\nThanks.\n"},{"id":"225467","messageId":"xmqqa9kd8uga.fsf@gitster.dls.corp.google.com","threadId":"34717","inReplyTo":"61B1EB04-497F-4398-8C52-CCE3A1A81B10@zib.de","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-19T17:24:37Z","receivedAt":"2013-08-19T17:24:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska writes:\n\n> I'm happy to rework it again towards your suggestion. I would also remove\n> the compat wrapper for write(). But I got a bit tired. I'd appreciate if\n> I received more indication whether a version without compat wrappers would\n> be accepted.\n\nI think it is a reasonable way forward to remove the writer side\nwrapper and doing large IO in reasonably big (but small enough not to\ntrigger MacOS X limitations) chunks in both read/write direction.\n\nLinus, thanks for a dose of sanity.\n"},{"id":"225468","messageId":"CA+55aFxYRspM+FNyXX8v7WTeCfSAzPdFWSYCzC16J3iJvygRhA@mail.gmail.com","threadId":"34717","inReplyTo":"xmqqeh9p8ut3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-08-19T17:28:03Z","receivedAt":"2013-08-19T17:28:03Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Aug 19, 2013 at 10:16 AM, Junio C Hamano wrote:\n> Linus Torvalds writes:\n>\n> The same argument applies to xwrite(), but currently we explicitly\n> catch EINTR and EAGAIN knowing that on sane systems these are the\n> signs that we got interrupted.\n>\n> Do we catch EINVAL unconditionally in the same codepath?\n\nNo, and we shouldn't. If EINVAL happens, it will keep happening.\n\nBut with the size limiter, it doesn't matter, since we won't hit the\nOS X braindamage.\n\n> Could\n> EINVAL on saner systems mean completely different thing (like our\n> caller is passing bogus parameters to underlying read/write, which\n> is a program bug we would want to catch)?\n\nYes. Even on OS X, it means that - it's just that OS X notion of what\nis \"bogus\" is pure crap. But the thing is, looping on EINVAL would be\nwrong even on OS X, since unless you change the size, it will keep\nhappening forever.\n\nBut with the \"limit IO to 8MB\" (or whatever) patch, the issue is moot.\nIf you get an EINVAL, it will be due to something else being horribly\nhorribly wrong.\n\n Linus\n"},{"id":"225485","messageId":"7E527329-230E-4954-9942-8BB0935ACE4D@gmail.com","threadId":"34717","inReplyTo":"xmqqeh9p8ut3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Kyle J. McKay","fromEmail":"mackyle@gmail.com","sentAt":"2013-08-19T21:56:00Z","receivedAt":"2013-08-19T21:56:00Z","isPatch":true,"sender":{"key":"mackyle@gmail.com","avatar":"https://avatars.githubusercontent.com/u/813346?v=4"},"body":"On Aug 19, 2013, at 10:16, Junio C Hamano wrote:\n\n> Linus Torvalds writes:\n>\n>> So why isn't the patch much more straightforward? Like the attached\n>> totally untested one that just limits the read/write size to 8MB\n>> (which is totally arbitrary, but small enough to not have any latency\n>> issues even on slow disks, and big enough that any reasonable IO\n>> subsystem will still get good throughput).\n>\n> Ahh. OK, not noticing EINVAL unconditionally, but always feed IOs\n> in chunks that are big enough for sane systems but small enough for\n> broken ones.\n>\n> That makes sense. Could somebody on MacOS X test this?\n\nI tested this on both i386 (OS X 32-bit intel) and x86_64 (OS X 64-bit \nintel).\n\nWhat I tested:\n\n1. I started with branch pu:\n (965adb10 Merge branch 'sg/bash-prompt-lf-in-cwd-test' into pu)\n\n2. I added Steffen's additional test (modified to always run) to t0021:\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,16 @@ test_expect_success 'required filter clean \nfailure' '\n\ttest_must_fail git add test.fc\n'\n\n+test_expect_success 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\ntest_done\n\n3. I verified that the test fails with an unpatched build on both 32- \nbit and 64-bit.\n\n4. I applied Linus's unmodified patch to wrapper.c.\n\n5. I tested again. The t0021 test now passes on 64-bit. It still \nfails on 32-bit for another reason unrelated to Linus's patch.\n\nIt fails when attempting the \"git add 2GB\" line from the new 'filter \nlarge file' part of the test. The failure with backtrace:\n\ngit(16806,0xa095c720) malloc: *** mmap(size=2147487744) failed (error \ncode=12)\n*** error: can't allocate region\n*** set a breakpoint in malloc_error_break to debug\n\n# NOTE: error code 12 is ENOMEM on OS X\n\nBreakpoint 1, 0x97f634b1 in malloc_error_break ()\n(gdb) bt\n#0 0x97f634b1 in malloc_error_break ()\n#1 0x97f5e49f in szone_error ()\n#2 0x97e8b876 in allocate_pages ()\n#3 0x97e8c062 in large_and_huge_malloc ()\n#4 0x97e831c8 in szone_malloc ()\n#5 0x97e82fb8 in malloc_zone_malloc ()\n#6 0x97e8c7b2 in realloc ()\n#7 0x00128abe in xrealloc (ptr=0x0, size=2147483649) at wrapper.c:100\n#8 0x00111a8c in strbuf_grow (sb=0xbfffe634, extra=2147483648) at \nstrbuf.c:74\n#9 0x00112bb9 in strbuf_read (sb=0xbfffe634, fd=6, hint=2548572518) \nat strbuf.c:349\n#10 0x0009b899 in apply_filter (path=, src=0x1000000 ' ' ..., \nlen=2147483648, dst=0xbfffe774, cmd=0x402980 \"cat\") at convert.c:407\n#11 0x0009c6f6 in convert_to_git (path=0x4028b4 \"2GB\", src=0x1000000 ' \n' ..., len=2147483648, dst=0xbfffe774, \nchecksafe=SAFE_CRLF_WARN) at convert.c:764\n#12 0x0010bb38 in index_mem (sha1=0x402330 \"\", buf=0x1000000, \nsize=2147483648, type=OBJ_BLOB, path=0x4028b4 \"2GB\", flags=1) at \nsha1_file.c:3044\n#13 0x0010bf57 in index_core [inlined] () at /private/var/tmp/src/git/ \nsha1_file.c:3101\n#14 0x0010bf57 in index_fd (sha1=0x402330 \"\", fd=5, st=0xbfffe900, \ntype=OBJ_BLOB, path=0x4028b4 \"2GB\", flags=1) at sha1_file.c:3139\n#15 0x0010c05e in index_path (sha1=0x402330 \"\", path=0x4028b4 \"2GB\", \nst=0xbfffe900, flags=1) at sha1_file.c:3157\n#16 0x000e82f4 in add_to_index (istate=0x1a8820, path=0x4028b4 \"2GB\", \nst=0xbfffe900, flags=0) at read-cache.c:665\n#17 0x000e87c8 in add_file_to_index (istate=0x1a8820, path=0x4028b4 \n\"2GB\", flags=0) at read-cache.c:694\n#18 0x0000440a in cmd_add (argc=, argv=0xbffff584, prefix=0x0) at builtin/add.c:299\n#19 0x00002e1f in run_builtin [inlined] () at /private/var/tmp/src/git/ \ngit.c:303\n#20 0x00002e1f in handle_internal_command (argc=2, argv=0xbffff584) at \ngit.c:466\n#21 0x000032d4 in run_argv [inlined] () at /private/var/tmp/src/git/ \ngit.c:512\n#22 0x000032d4 in main (argc=2, av=0xbfffe28c) at git.c:595\n\nThe size 2147487744 is 2GB + 4096 bytes. Apparently git does not \nsupport a filter for a file unless the file can fit entirely into \ngit's memory space. Normally a single 2GB + 4096 byte allocation \nworks in an OS X 32-bit process, but something else is apparently \neating up a large portion of the memory space in this case (perhaps an \nmmap'd copy?). In any case, if the file being filtered was closer to \n4GB in size it would always fail on 32-bit regardless.\n\nThe fact that the entire file is read into memory when applying the \nfilter does not seem like a good thing (see #7-#10 above).\n\n--Kyle\n"},{"id":"225487","messageId":"CA+55aFzAQjxB7HkDqR6_3wdex1t1Tbrf5CeUVyiVm=DRyDVhhQ@mail.gmail.com","threadId":"34717","inReplyTo":"7E527329-230E-4954-9942-8BB0935ACE4D@gmail.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2013-08-19T22:51:41Z","receivedAt":"2013-08-19T22:51:41Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Mon, Aug 19, 2013 at 2:56 PM, Kyle J. McKay wrote:\n>\n> The fact that the entire file is read into memory when applying the filter\n> does not seem like a good thing (see #7-#10 above).\n\nYeah, that's horrible. Its likely bad for performance too, because\neven if you have enough memory, it blows everything out of the L2/L3\ncaches, and if you don't have enough memory it obviously causes other\nproblems.\n\nSo it would probably be a great idea to make the filtering code able\nto do things in smaller chunks, but I suspect that the patch to chunk\nup xread/xwrite is the right thing to do anyway.\n\n Linus\n"},{"id":"225497","messageId":"1376981035-23284-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376926879-30846-1-git-send-email-prohaska@zib.de","subject":"[PATCH v5 0/2] Fix IO of >=2GB on Mac OS X by limiting IO chunks","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-20T06:43:53Z","receivedAt":"2013-08-20T06:43:53Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"This is the revised patch taking the considerations about IO chunk size into\naccount. The series deletes more than it adds and fixes a bug. Nice.\n\nSteffen Prohaska (2):\n xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X\n Revert \"compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\"\n\n Makefile | 8 --------\n compat/clipped-write.c | 13 -------------\n config.mak.uname | 1 -\n git-compat-util.h | 5 -----\n t/t0021-conversion.sh | 14 ++++++++++++++\n wrapper.c | 12 ++++++++++++\n 6 files changed, 26 insertions(+), 27 deletions(-)\n delete mode 100644 compat/clipped-write.c\n\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225495","messageId":"1376981035-23284-2-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376981035-23284-1-git-send-email-prohaska@zib.de","subject":"[PATCH v5 1/2] xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-20T06:43:54Z","receivedAt":"2013-08-20T06:43:54Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering 2GB or more through an external filter (see test)\nfailed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason was that read() immediately returns with EINVAL if nbyte >=\n2GB. According to POSIX [1], if the value of nbyte passed to read() is\ngreater than SSIZE_MAX, the result is implementation-defined. The write\nfunction has the same restriction [2]. Since OS X still supports\nrunning 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX\n= 2GB - 1) seems to be also imposed on 64-bit executables under certain\nconditions. For write, the problem has been addressed earlier [6c642a].\n\nThis commit addresses the problem for read() and write() by limiting\nsize of IO chunks unconditionally on all platforms in xread() and\nxwrite(). Large chunks only cause problems, like triggering the OS\nX bug or causing latencies when killing the process. Reasonably sized\nsmaller chunks have no negative impact on performance.\n\nThe compat wrapper clipped_write() introduced earlier [6c642a] is not\nneeded anymore. It will be reverted in a separate commit. The new test\ncatches read and write problems.\n\nNote that 'git add' exits with 0 even if it prints filtering errors to\nstderr. The test, therefore, checks stderr. 'git add' should probably\nbe changed (sometime in another commit) to exit with nonzero if\nfiltering fails. The test could then be changed to use test_must_fail.\n\nThanks to the following people for suggestions and testing:\n\n Johannes Sixt \n John Keeping \n Jonathan Nieder \n Kyle J. McKay \n Linus Torvalds \n Torsten Bögershausen \n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n\n[6c642a] commit 6c642a878688adf46b226903858b53e2d31ac5c3\n compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\n\nSigned-off-by: Steffen Prohaska \n---\n t/t0021-conversion.sh | 14 ++++++++++++++\n wrapper.c | 12 ++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\n test_done\ndiff --git a/wrapper.c b/wrapper.c\nindex 6a015de..97e3cf7 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -131,6 +131,14 @@ void *xcalloc(size_t nmemb, size_t size)\n }\n \n /*\n+ * Limit size of IO chunks, because huge chunks only cause pain. OS X 64-bit\n+ * buggy, returning EINVAL if len >= INT_MAX; and even in the absense of bugs,\n+ * large chunks can result in bad latencies when you decide to kill the\n+ * process.\n+ */\n+#define MAX_IO_SIZE (8*1024*1024)\n+\n+/*\n * xread() is the same a read(), but it automatically restarts read()\n * operations with a recoverable error (EAGAIN and EINTR). xread()\n * DOES NOT GUARANTEE that \"len\" bytes is read even if the data is available.\n@@ -138,6 +146,8 @@ void *xcalloc(size_t nmemb, size_t size)\n ssize_t xread(int fd, void *buf, size_t len)\n {\n \tssize_t nr;\n+\tif (len > MAX_IO_SIZE)\n+\t len = MAX_IO_SIZE;\n \twhile (1) {\n \t\tnr = read(fd, buf, len);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n@@ -154,6 +164,8 @@ ssize_t xread(int fd, void *buf, size_t len)\n ssize_t xwrite(int fd, const void *buf, size_t len)\n {\n \tssize_t nr;\n+\tif (len > MAX_IO_SIZE)\n+\t len = MAX_IO_SIZE;\n \twhile (1) {\n \t\tnr = write(fd, buf, len);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225496","messageId":"1376981035-23284-3-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376981035-23284-1-git-send-email-prohaska@zib.de","subject":"[PATCH v5 2/2] Revert \"compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\"","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-20T06:43:55Z","receivedAt":"2013-08-20T06:43:55Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"The previous commit introduced a size limit on IO chunks on all\nplatforms. The compat clipped_write() is not needed anymore.\n\nThis reverts commit 6c642a878688adf46b226903858b53e2d31ac5c3.\n\nSigned-off-by: Steffen Prohaska \n---\n Makefile | 8 --------\n compat/clipped-write.c | 13 -------------\n config.mak.uname | 1 -\n git-compat-util.h | 5 -----\n 4 files changed, 27 deletions(-)\n delete mode 100644 compat/clipped-write.c\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..4026211 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,9 +69,6 @@ all::\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n-# Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n-# INT_MAX bytes at once (e.g. MacOS X).\n-#\n # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH\n # it specifies.\n #\n@@ -1493,11 +1490,6 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n-ifdef NEEDS_CLIPPED_WRITE\n-\tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n-\tCOMPAT_OBJS += compat/clipped-write.o\n-endif\n-\n ifneq (,$(XDL_FAST_HASH))\n \tBASIC_CFLAGS += -DXDL_FAST_HASH\n endif\ndiff --git a/compat/clipped-write.c b/compat/clipped-write.c\ndeleted file mode 100644\nindex b8f98ff..0000000\n--- a/compat/clipped-write.c\n+++ /dev/null\n@@ -1,13 +0,0 @@\n-#include \"../git-compat-util.h\"\n-#undef write\n-\n-/*\n- * Version of write that will write at most INT_MAX bytes.\n- * Workaround a xnu bug on Mac OS X\n- */\n-ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)\n-{\n-\tif (nbyte > INT_MAX)\n-\t\tnbyte = INT_MAX;\n-\treturn write(fildes, buf, nbyte);\n-}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex b27f51d..7d61531 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -95,7 +95,6 @@ ifeq ($(uname_S),Darwin)\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n-\tNEEDS_CLIPPED_WRITE = YesPlease\n \tCOMPAT_OBJS += compat/precompose_utf8.o\n \tBASIC_CFLAGS += -DPRECOMPOSE_UNICODE\n endif\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 115cb1d..96d8881 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -185,11 +185,6 @@ typedef unsigned long uintptr_t;\n #define probe_utf8_pathname_composition(a,b)\n #endif\n \n-#ifdef NEEDS_CLIPPED_WRITE\n-ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);\n-#define write(x,y,z) clipped_write((x),(y),(z))\n-#endif\n-\n #ifdef MKDIR_WO_TRAILING_SLASH\n #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))\n extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225551","messageId":"xmqqli3w5f1u.fsf@gitster.dls.corp.google.com","threadId":"34717","inReplyTo":"1376981035-23284-2-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v5 1/2] xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-20T19:37:49Z","receivedAt":"2013-08-20T19:37:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska writes:\n\n> diff --git a/wrapper.c b/wrapper.c\n> index 6a015de..97e3cf7 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -131,6 +131,14 @@ void *xcalloc(size_t nmemb, size_t size)\n> }\n> \n> /*\n> + * Limit size of IO chunks, because huge chunks only cause pain. OS X 64-bit\n> + * buggy, returning EINVAL if len >= INT_MAX; and even in the absense of bugs,\n\ns/buggy/is &/ perhaps?\n\n> + * large chunks can result in bad latencies when you decide to kill the\n> + * process.\n> + */\n> +#define MAX_IO_SIZE (8*1024*1024)\n> +\n> +/*\n> * xread() is the same a read(), but it automatically restarts read()\n> * operations with a recoverable error (EAGAIN and EINTR). xread()\n> * DOES NOT GUARANTEE that \"len\" bytes is read even if the data is available.\n> @@ -138,6 +146,8 @@ void *xcalloc(size_t nmemb, size_t size)\n> ssize_t xread(int fd, void *buf, size_t len)\n> {\n> \tssize_t nr;\n> +\tif (len > MAX_IO_SIZE)\n> +\t len = MAX_IO_SIZE;\n> \twhile (1) {\n> \t\tnr = read(fd, buf, len);\n> \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n> @@ -154,6 +164,8 @@ ssize_t xread(int fd, void *buf, size_t len)\n> ssize_t xwrite(int fd, const void *buf, size_t len)\n> {\n> \tssize_t nr;\n> +\tif (len > MAX_IO_SIZE)\n> +\t len = MAX_IO_SIZE;\n> \twhile (1) {\n> \t\tnr = write(fd, buf, len);\n> \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n"},{"id":"225614","messageId":"1377092782-11924-1-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1376981035-23284-1-git-send-email-prohaska@zib.de","subject":"[PATCH v6 0/2] Fix IO >= 2GB on Mac, fixed typo","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-21T13:46:20Z","receivedAt":"2013-08-21T13:46:20Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Fixed typo in comment.\n\nSteffen Prohaska (2):\n xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X\n Revert \"compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\"\n\n Makefile | 8 --------\n compat/clipped-write.c | 13 -------------\n config.mak.uname | 1 -\n git-compat-util.h | 5 -----\n t/t0021-conversion.sh | 14 ++++++++++++++\n wrapper.c | 12 ++++++++++++\n 6 files changed, 26 insertions(+), 27 deletions(-)\n delete mode 100644 compat/clipped-write.c\n\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225615","messageId":"1377092782-11924-2-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1377092782-11924-1-git-send-email-prohaska@zib.de","subject":"[PATCH v5 1/2] xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-21T13:46:21Z","receivedAt":"2013-08-21T13:46:21Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"Previously, filtering 2GB or more through an external filter (see test)\nfailed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:\n\n error: read from external filter cat failed\n error: cannot feed the input to external filter cat\n error: cat died of signal 13\n error: external filter cat failed 141\n error: external filter cat failed\n\nThe reason was that read() immediately returns with EINVAL if nbyte >=\n2GB. According to POSIX [1], if the value of nbyte passed to read() is\ngreater than SSIZE_MAX, the result is implementation-defined. The write\nfunction has the same restriction [2]. Since OS X still supports\nrunning 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX\n= 2GB - 1) seems to be also imposed on 64-bit executables under certain\nconditions. For write, the problem has been addressed earlier [6c642a].\n\nThis commit addresses the problem for read() and write() by limiting\nsize of IO chunks unconditionally on all platforms in xread() and\nxwrite(). Large chunks only cause problems, like triggering the OS\nX bug or causing latencies when killing the process. Reasonably sized\nsmaller chunks have no negative impact on performance.\n\nThe compat wrapper clipped_write() introduced earlier [6c642a] is not\nneeded anymore. It will be reverted in a separate commit. The new test\ncatches read and write problems.\n\nNote that 'git add' exits with 0 even if it prints filtering errors to\nstderr. The test, therefore, checks stderr. 'git add' should probably\nbe changed (sometime in another commit) to exit with nonzero if\nfiltering fails. The test could then be changed to use test_must_fail.\n\nThanks to the following people for suggestions and testing:\n\n Johannes Sixt \n John Keeping \n Jonathan Nieder \n Kyle J. McKay \n Linus Torvalds \n Torsten Bögershausen \n\n[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html\n[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html\n\n[6c642a] commit 6c642a878688adf46b226903858b53e2d31ac5c3\n compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\n\nSigned-off-by: Steffen Prohaska \n---\n t/t0021-conversion.sh | 14 ++++++++++++++\n wrapper.c | 12 ++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh\nindex e50f0f7..b92e6cb 100755\n--- a/t/t0021-conversion.sh\n+++ b/t/t0021-conversion.sh\n@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '\n \ttest_must_fail git add test.fc\n '\n \n+test -n \"$GIT_TEST_LONG\" && test_set_prereq EXPENSIVE\n+\n+test_expect_success EXPENSIVE 'filter large file' '\n+\tgit config filter.largefile.smudge cat &&\n+\tgit config filter.largefile.clean cat &&\n+\tfor i in $(test_seq 1 2048); do printf \"%1048576d\" 1; done >2GB &&\n+\techo \"2GB filter=largefile\" >.gitattributes &&\n+\tgit add 2GB 2>err &&\n+\t! test -s err &&\n+\trm -f 2GB &&\n+\tgit checkout -- 2GB 2>err &&\n+\t! test -s err\n+'\n+\n test_done\ndiff --git a/wrapper.c b/wrapper.c\nindex 6a015de..66cc727 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -131,6 +131,14 @@ void *xcalloc(size_t nmemb, size_t size)\n }\n \n /*\n+ * Limit size of IO chunks, because huge chunks only cause pain. OS X 64-bit\n+ * is buggy, returning EINVAL if len >= INT_MAX; and even in the absense of\n+ * bugs, large chunks can result in bad latencies when you decide to kill the\n+ * process.\n+ */\n+#define MAX_IO_SIZE (8*1024*1024)\n+\n+/*\n * xread() is the same a read(), but it automatically restarts read()\n * operations with a recoverable error (EAGAIN and EINTR). xread()\n * DOES NOT GUARANTEE that \"len\" bytes is read even if the data is available.\n@@ -138,6 +146,8 @@ void *xcalloc(size_t nmemb, size_t size)\n ssize_t xread(int fd, void *buf, size_t len)\n {\n \tssize_t nr;\n+\tif (len > MAX_IO_SIZE)\n+\t len = MAX_IO_SIZE;\n \twhile (1) {\n \t\tnr = read(fd, buf, len);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n@@ -154,6 +164,8 @@ ssize_t xread(int fd, void *buf, size_t len)\n ssize_t xwrite(int fd, const void *buf, size_t len)\n {\n \tssize_t nr;\n+\tif (len > MAX_IO_SIZE)\n+\t len = MAX_IO_SIZE;\n \twhile (1) {\n \t\tnr = write(fd, buf, len);\n \t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225616","messageId":"1377092782-11924-3-git-send-email-prohaska@zib.de","threadId":"34717","inReplyTo":"1377092782-11924-1-git-send-email-prohaska@zib.de","subject":"[PATCH v5 2/2] Revert \"compate/clipped-write.c: large write(2) fails on Mac OS X/XNU\"","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2013-08-21T13:46:22Z","receivedAt":"2013-08-21T13:46:22Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"The previous commit introduced a size limit on IO chunks on all\nplatforms. The compat clipped_write() is not needed anymore.\n\nThis reverts commit 6c642a878688adf46b226903858b53e2d31ac5c3.\n\nSigned-off-by: Steffen Prohaska \n---\n Makefile | 8 --------\n compat/clipped-write.c | 13 -------------\n config.mak.uname | 1 -\n git-compat-util.h | 5 -----\n 4 files changed, 27 deletions(-)\n delete mode 100644 compat/clipped-write.c\n\ndiff --git a/Makefile b/Makefile\nindex 3588ca1..4026211 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -69,9 +69,6 @@ all::\n # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt\n # doesn't support GNU extensions like --check and --statistics\n #\n-# Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than\n-# INT_MAX bytes at once (e.g. MacOS X).\n-#\n # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH\n # it specifies.\n #\n@@ -1493,11 +1490,6 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS\n \tMSGFMT += --check --statistics\n endif\n \n-ifdef NEEDS_CLIPPED_WRITE\n-\tBASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE\n-\tCOMPAT_OBJS += compat/clipped-write.o\n-endif\n-\n ifneq (,$(XDL_FAST_HASH))\n \tBASIC_CFLAGS += -DXDL_FAST_HASH\n endif\ndiff --git a/compat/clipped-write.c b/compat/clipped-write.c\ndeleted file mode 100644\nindex b8f98ff..0000000\n--- a/compat/clipped-write.c\n+++ /dev/null\n@@ -1,13 +0,0 @@\n-#include \"../git-compat-util.h\"\n-#undef write\n-\n-/*\n- * Version of write that will write at most INT_MAX bytes.\n- * Workaround a xnu bug on Mac OS X\n- */\n-ssize_t clipped_write(int fildes, const void *buf, size_t nbyte)\n-{\n-\tif (nbyte > INT_MAX)\n-\t\tnbyte = INT_MAX;\n-\treturn write(fildes, buf, nbyte);\n-}\ndiff --git a/config.mak.uname b/config.mak.uname\nindex b27f51d..7d61531 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -95,7 +95,6 @@ ifeq ($(uname_S),Darwin)\n \tNO_MEMMEM = YesPlease\n \tUSE_ST_TIMESPEC = YesPlease\n \tHAVE_DEV_TTY = YesPlease\n-\tNEEDS_CLIPPED_WRITE = YesPlease\n \tCOMPAT_OBJS += compat/precompose_utf8.o\n \tBASIC_CFLAGS += -DPRECOMPOSE_UNICODE\n endif\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 115cb1d..96d8881 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -185,11 +185,6 @@ typedef unsigned long uintptr_t;\n #define probe_utf8_pathname_composition(a,b)\n #endif\n \n-#ifdef NEEDS_CLIPPED_WRITE\n-ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);\n-#define write(x,y,z) clipped_write((x),(y),(z))\n-#endif\n-\n #ifdef MKDIR_WO_TRAILING_SLASH\n #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))\n extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);\n-- \n1.8.4.rc3.5.g4f480ff\n"},{"id":"225617","messageId":"xmqq8uzvujcj.fsf@gitster.dls.corp.google.com","threadId":"34717","inReplyTo":"1377092782-11924-1-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v6 0/2] Fix IO >= 2GB on Mac, fixed typo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-21T15:58:04Z","receivedAt":"2013-08-21T15:58:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steffen Prohaska writes:\n\n> Fixed typo in comment.\n\nThanks, and sorry for not being clear that I'll locally tweak before\nqueuing when I commented on v5 yesterday.\n"},{"id":"225634","messageId":"52151A08.6060103@web.de","threadId":"34717","inReplyTo":"1376981035-23284-2-git-send-email-prohaska@zib.de","subject":"Re: [PATCH v5 1/2] xread, xwrite: Limit size of IO, fixing IO of 2GB and more on Mac OS X","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-08-21T19:50:32Z","receivedAt":"2013-08-21T19:50:32Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-08-20 08.43, Steffen Prohaska wrote:\n[]\nThanks for V5. It was tested OK on my system here.\n(And apologies for recommending a wrapper on top of a wrapper).\n\nOne question is left: \nAs xread() is tolerant against EAGAIN and especially EINTR,\ncould it make sense to replace read() with xread() everywhere?\n\n(The risk for getting EINTR is smaller when we only read a small amount\nof data, but it is more on the safe side)\n\nAnd s/write/xwrite/\n\n/Torsten\n"},{"id":"225980","messageId":"xmqq4nabhgpp.fsf@gitster.dls.corp.google.com","threadId":"34717","inReplyTo":"CA+55aFzAQjxB7HkDqR6_3wdex1t1Tbrf5CeUVyiVm=DRyDVhhQ@mail.gmail.com","subject":"Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-27T04:59:14Z","receivedAt":"2013-08-27T04:59:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> So it would probably be a great idea to make the filtering code able\n> to do things in smaller chunks, but I suspect that the patch to chunk\n> up xread/xwrite is the right thing to do anyway.\n\nYes and yes, but the first yes is a bit tricky for writing things\nout, as the recipient of the filter knows the size of the input but\nnot of the output, and both loose and packed objects needs to record\nthe length of the object at the very beginning.\n\nEven though our streaming API allows to write new objects directly\nto a packfile, for user-specified filters, CRLF, and ident can make\nthe size of the output unknown before processing all the data, so\nthe best we could do for these would be to stream to a temporary\nfile and then copy it again with the length header (undeltified\npacked object deflates only the payload, so this \"copy\" can\nliterally be a byte-for-byte copy, after writing the in-pack header\nout).\n\nAs reading from the object store and writing it out to the\nfilesystem (i.e. entry.c::write_entry() codepath) does not need to\nknow the output size, convert.c::get_stream_filter() might want to\nbe told in which direction a filter is asked for and return a\nstreaming filter back even when those filters that are problematic\nfor the opposite, writing-to-object-store direction.\n"}]}