Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-26 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes: So it would probably be a great idea to make the filtering code able to do things in smaller chunks, but I suspect that the patch to chunk up xread/xwrite is the right thing to do anyway. Yes and yes, but the first yes is a bit tricky for

[PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Steffen Prohaska
Previously, filtering 2GB or more through an external filter (see test) failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with: error: read from external filter cat failed error: cannot feed the input to external filter cat error: cat died of signal 13 error: external

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Linus Torvalds
On Mon, Aug 19, 2013 at 8:41 AM, Steffen Prohaska proha...@zib.de wrote: The reason was that read() immediately returns with EINVAL if nbyte = 2GB. According to POSIX [1], if the value of nbyte passed to read() is greater than SSIZE_MAX, the result is implementation-defined. Yeah, the OS X

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Steffen Prohaska
On Aug 19, 2013, at 6:04 PM, Linus Torvalds torva...@linux-foundation.org wrote: I hate your patch for other reasons, though: The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Why do you do that? We already

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes: I hate your patch for other reasons, though: The problem for read() is addressed in a similar way by introducing a wrapper function in compat that always reads less than 2GB. Why do you do that? We already _have_ wrapper functions for

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Junio C Hamano
Steffen Prohaska proha...@zib.de writes: I'm happy to rework it again towards your suggestion. I would also remove the compat wrapper for write(). But I got a bit tired. I'd appreciate if I received more indication whether a version without compat wrappers would be accepted. I think it is

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Linus Torvalds
On Mon, Aug 19, 2013 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote: Linus Torvalds torva...@linux-foundation.org writes: The same argument applies to xwrite(), but currently we explicitly catch EINTR and EAGAIN knowing that on sane systems these are the signs that we got interrupted.

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Kyle J. McKay
On Aug 19, 2013, at 10:16, Junio C Hamano wrote: Linus Torvalds torva...@linux-foundation.org writes: So why isn't the patch much more straightforward? Like the attached totally untested one that just limits the read/write size to 8MB (which is totally arbitrary, but small enough to not have

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

2013-08-19 Thread Linus Torvalds
On Mon, Aug 19, 2013 at 2:56 PM, Kyle J. McKay mack...@gmail.com wrote: The fact that the entire file is read into memory when applying the filter does not seem like a good thing (see #7-#10 above). Yeah, that's horrible. Its likely bad for performance too, because even if you have enough