read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Hi there

While investigating the problem with hung git-upload-pack we think to have 
found a bug in wrapper.c:

#define MAX_IO_SIZE (8*1024*1024)

This is then used in xread() to split read()s into suitable chunks.
So far so good, but read() is only guaranteed to read as much as SSIZE_MAX 
bytes at a time. And on our platform that is way lower than those 8MB (only 
52kB, POSIX allows it to be as small as 32k), and as a (rather strange) 
consequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), 
because xpread() returns something > 0.

How large is SSIZE_MAX on other platforms? What happens there if you try to 
read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
reading the header files right, on Linux it is LONG_MAX (2TB?), so I guess 
we should really go for MIN(8*1024*1024,SSIZE_MAX)?


bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:

> because xpread() returns something > 0.
 something < 0 of course (presumably -1)...

bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Torsten Bögershausen]

On 2015-02-07 17.45, Joachim Schmitz wrote:
> Hi there
> 
> While investigating the problem with hung git-upload-pack we think to have 
> found a bug in wrapper.c:
> 
> #define MAX_IO_SIZE (8*1024*1024)
> 
> This is then used in xread() to split read()s into suitable chunks.
> So far so good, but read() is only guaranteed to read as much as SSIZE_MAX 
> bytes at a time. And on our platform that is way lower than those 8MB (only 
> 52kB, POSIX allows it to be as small as 32k), and as a (rather strange) 
> consequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), 
> because xpread() returns something > 0.
> 
> How large is SSIZE_MAX on other platforms? What happens there if you try to 
> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
> reading the header files right, on Linux it is LONG_MAX (2TB?), so I guess 
> we should really go for MIN(8*1024*1024,SSIZE_MAX)?

How about changing wrapper.c like this:

#ifndef MAX_IO_SIZE
 #define MAX_IO_SIZE (8*1024*1024)
#endif
---------------------
and to change config.mak.uname like this:

ifeq ($(uname_S),NONSTOP_KERNEL)

	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
Does this work for you ?

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Torsten Bögershausen <tboegi <at> web.de> writes:

> 
> On 2015-02-07 17.45, Joachim Schmitz wrote:
<snip>
> 
> How about changing wrapper.c like this:
> 
> #ifndef MAX_IO_SIZE
>  #define MAX_IO_SIZE (8*1024*1024)
> #endif
> ---------------------
> and to change config.mak.uname like this:
> 
> ifeq ($(uname_S),NONSTOP_KERNEL)
> 
> 	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
> Does this work for you ?

Of course it would, but, 
a) 32k is smaller than we can go (and yes, we could make it 52k)
b) never ever should read() be asked to read more than SSIZE_MAX, this  
should be true for every platform on the planet? You may want to have is 
smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on 
Linux), but surely never larger?

Bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:

> 
> Torsten Bögershausen <tboegi <at> web.de> writes:
> 
> > 
> > On 2015-02-07 17.45, Joachim Schmitz wrote:
<snip>
> b) never ever should read() be asked to read more than SSIZE_MAX, this  
> should be true for every platform on the planet? You may want to have is 
> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on 
> Linux), but surely never larger?

Se also $gmane/232469, where that issue cropped up for MacOS X 64bit?

Bye, Jojo

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Randall S. Becker]

On 2015-02-07 12:30PM Torsten Bögershausen wrote:
>On 2015-02-07 17.45, Joachim Schmitz wrote:
>> Hi there
>> 
>> While investigating the problem with hung git-upload-pack we think to 
>> have found a bug in wrapper.c:
>> 
>> #define MAX_IO_SIZE (8*1024*1024)
>> 
>> This is then used in xread() to split read()s into suitable chunks.
>> So far so good, but read() is only guaranteed to read as much as 
>> SSIZE_MAX bytes at a time. And on our platform that is way lower than 
>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a 
>> (rather strange) consequence mmap() (from compat/mmap.c) fails with 
>> EACCESS (why EACCESS?), because xpread() returns something > 0.
>> 
>> How large is SSIZE_MAX on other platforms? What happens there if you 
>> try to
>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I 
>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?
>How about changing wrapper.c like this: 
>#ifndef MAX_IO_SIZE
> #define MAX_IO_SIZE (8*1024*1024)
>#endif
>---------------------
>and to change config.mak.uname like this:
>ifeq ($(uname_S),NONSTOP_KERNEL)
>	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024) Does this work for you ?

Yes, thank you Torsten. I have made this change in our branch (on behalf of
Jojo). I think we can accept it. The (32*1024) does need to be properly
quoted, however.

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Randall S. Becker]

On 2015-02-07 13:07PM Randall S. Becker wrote:
>On 2015-02-07 12:30PM Torsten Bögershausen wrote:
>>On 2015-02-07 17.45, Joachim Schmitz wrote:
>>> Hi there
>>> 
>>> While investigating the problem with hung git-upload-pack we think to 
>>> have found a bug in wrapper.c:
>>> 
>>> #define MAX_IO_SIZE (8*1024*1024)
>>> 
>>> This is then used in xread() to split read()s into suitable chunks.
>>> So far so good, but read() is only guaranteed to read as much as 
>>> SSIZE_MAX bytes at a time. And on our platform that is way lower than 
>>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a 
>>> (rather strange) consequence mmap() (from compat/mmap.c) fails with 
>>> EACCESS (why EACCESS?), because xpread() returns something > 0.
>>> 
>>> How large is SSIZE_MAX on other platforms? What happens there if you 
>>> try to
>>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm 
>>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I 
>>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?
>>How about changing wrapper.c like this: 
>>#ifndef MAX_IO_SIZE
>> #define MAX_IO_SIZE (8*1024*1024)
>>#endif
>>---------------------

Although I do agree with Jojo, that MAX_IO_SIZE seems to be a platform
constant and should be defined in terms of SSIZE_MAX. So something like:

#ifndef MAX_IO_SIZE
# ifdef SSIZE_MAX
#  define MAX_IO_SIZE (SSIZE_MAX)
# else
#  define MAX_IO_SIZE (8*1024*1024)
# endif
#endif

would be desirable.

Cheers, Randall

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Randall S. Becker <rsbecker <at> nexbridge.com> writes:

> 
> On 2015-02-07 13:07PM Randall S. Becker wrote:
> >On 2015-02-07 12:30PM Torsten Bögershausen wrote:
> >>On 2015-02-07 17.45, Joachim Schmitz wrote:
<spip> 
> Although I do agree with Jojo, that MAX_IO_SIZE seems to be a platform
> constant and should be defined in terms of SSIZE_MAX. So something like:
> 
> #ifndef MAX_IO_SIZE
> # ifdef SSIZE_MAX
> #  define MAX_IO_SIZE (SSIZE_MAX)
> # else
> #  define MAX_IO_SIZE (8*1024*1024)
> # endif
> #endif
> 
> would be desirable.

It would be way too large on some platforms. those 8MB had been chosen for 
a good reason, I assume:

/*
 * Limit size of IO chunks, because huge chunks only cause pain.  OS X
 * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
 * the absence of bugs, large chunks can result in bad latencies when
 * you decide to kill the process.
 */

However it should never be larger than SSIZE_MAX

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:

<snip>
> and as a (rather strange) 
> consequence mmap() (from compat/mmap.c) fails with EACCESS (why 
EACCESS?), 
> because xpread() returns something > 0.

Seems mmap() should either set errno to EINVAL or not set it at all an 
just 'forward' whatever xpread() has set.

As per http://man7.org/linux/man-pages/man2/mmap.2.html mmap() sets EINVAL 
if (amongst other things) it doesn't like the value of len, exactly the 
case here.

bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Torsten Bögershausen]

On 2015-02-07 18.29, Joachim Schmitz wrote:
> Torsten Bögershausen <tboegi <at> web.de> writes:
> 
>>
>> On 2015-02-07 17.45, Joachim Schmitz wrote:
> <snip>
>>
>> How about changing wrapper.c like this:
>>
>> #ifndef MAX_IO_SIZE
>>  #define MAX_IO_SIZE (8*1024*1024)
>> #endif
>> ---------------------
>> and to change config.mak.uname like this:
>>
>> ifeq ($(uname_S),NONSTOP_KERNEL)
>>
>> 	BASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)
>> Does this work for you ?
> 
> Of course it would, but, 
> a) 32k is smaller than we can go (and yes, we could make it 52k)
Sorry, I missed that:  (52*1024)
> b) never ever should read() be asked to read more than SSIZE_MAX, this  
> should be true for every platform on the planet? You may want to have is 
> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on 
> Linux), but surely never larger?
> 
Good question.
I don't know every platform of the planet well enough to be helpful here,
especially the ones which don't follow all the specifications.

In other words: As long as we can not guarantee that SSIZE_MAX is defined,
(and is defined to somethong useful for xread()/xwrite() )
we should be more defensive here:

tweak only on platform where we know it is needed and we know that it works.

And leave the other ones alone, until someone finds another
platform which needs the same or another tweak and sends a tested patch.


Thanks for the report, do you want to send a patch to the list ?

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Junio C Hamano]

On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> I don't know every platform of the planet well enough to be helpful here,
> especially the ones which don't follow all the specifications.
>
> In other words: As long as we can not guarantee that SSIZE_MAX is defined,
> (and is defined to somethong useful for xread()/xwrite() )
> we should be more defensive here:
>
> tweak only on platform where we know it is needed and we know that it works.

Yup, I agree that is a sensible way to go.

 (1) if Makefile overrides the size, use it; otherwise
 (2) if SSIZE_MAX is defined, and it is smaller than our internal
default, use it; otherwise
 (3) use our internal default.

And leave our internal default to 8MB.

That way, nobody needs to do anything differently from his current build set-up,
and I suspect that it would make step (1) optional.

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Junio C Hamano <gitster <at> pobox.com> writes:

> 
> On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi <at> 
web.de> wrote:
> > I don't know every platform of the planet well enough to be helpful 
here,
> > especially the ones which don't follow all the specifications.
> >
> > In other words: As long as we can not guarantee that SSIZE_MAX is 
defined,
> > (and is defined to somethong useful for xread()/xwrite() )
> > we should be more defensive here:
> >
> > tweak only on platform where we know it is needed and we know that it 
works.
> 
> Yup, I agree that is a sensible way to go.
> 
>  (1) if Makefile overrides the size, use it; otherwise
>  (2) if SSIZE_MAX is defined, and it is smaller than our internal
> default, use it; otherwise
>  (3) use our internal default.
> 
> And leave our internal default to 8MB.
> 
> That way, nobody needs to do anything differently from his current build 
set-up,
> and I suspect that it would make step (1) optional.
> 

something like this:

/* allow overwriting from e.g. Makefile */
#if !defined(MAX_IO_SIZE)
# define MAX_IO_SIZE (8*1024*1024)
#endif
/* for plattforms that have SSIZE and have it smaller */
#if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) 
# undef MAX_IO_SIZE /* avoid warning */
# define MAX_IO_SIZE SSIZE_MAX
#endif

Steps 2 and 3 only , indeed step 1 not needed...

Bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Junio C Hamano]

On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
> Junio C Hamano <gitster <at> pobox.com> writes:
>>
>> Yup, I agree that is a sensible way to go.
>>
>>  (1) if Makefile overrides the size, use it; otherwise
>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal
>> default, use it; otherwise
>>  (3) use our internal default.
>>
>> And leave our internal default to 8MB.
>>
>> That way, nobody needs to do anything differently from his current build
> set-up,
>> and I suspect that it would make step (1) optional.
>
> something like this:
>
> /* allow overwriting from e.g. Makefile */
> #if !defined(MAX_IO_SIZE)
> # define MAX_IO_SIZE (8*1024*1024)
> #endif
> /* for plattforms that have SSIZE and have it smaller */
> #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)
> # undef MAX_IO_SIZE /* avoid warning */
> # define MAX_IO_SIZE SSIZE_MAX
> #endif

No, not like that. If you do (1), that is only so that the Makefile can override
a broken definition a platform may give to SSIZE_MAX.  So

 (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
 (2) if SSIZE_MAX is defined, and if it is smaller than our internal
default, use it.
 (3) all other cases, us our internal default.

RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Randall S. Becker]

On Feb 7 2015 at 9:14 PM Junio C Hamano wrote:
>On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
>> Junio C Hamano <gitster <at> pobox.com> writes:
>>>
>>> Yup, I agree that is a sensible way to go.
>>>
>>>  (1) if Makefile overrides the size, use it; otherwise
>>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal 
>>> default, use it; otherwise
>>>  (3) use our internal default.
>>>
>>> And leave our internal default to 8MB.
>>>
>>> That way, nobody needs to do anything differently from his current 
>>> build
>> set-up,
>>> and I suspect that it would make step (1) optional.
>>
>> something like this:
>>
>> /* allow overwriting from e.g. Makefile */ #if !defined(MAX_IO_SIZE) # 
>> define MAX_IO_SIZE (8*1024*1024) #endif
>> /* for plattforms that have SSIZE and have it smaller */ #if 
>> defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) # undef MAX_IO_SIZE /* 
>> avoid warning */ # define MAX_IO_SIZE SSIZE_MAX #endif
>No, not like that. If you do (1), that is only so that the Makefile can override a broken definition a platform may give to SSIZE_MAX.  So
> (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
> (2) if SSIZE_MAX is defined, and if it is smaller than our internal default, use it.
> (3) all other cases, us our internal default.

That is reasonable. I am more concerned about our git-upload-pak (separate thread) anyway :)

Cheers, Randall

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Junio C Hamano <gitster <at> pobox.com> writes:

<snip>
> >
> > something like this:
> >
> > /* allow overwriting from e.g. Makefile */
> > #if !defined(MAX_IO_SIZE)
> > # define MAX_IO_SIZE (8*1024*1024)
> > #endif
> > /* for plattforms that have SSIZE and have it smaller */
> > #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)
> > # undef MAX_IO_SIZE /* avoid warning */
> > # define MAX_IO_SIZE SSIZE_MAX
> > #endif
> 
> No, not like that. If you do (1), that is only so that the Makefile can 
override
> a broken definition a platform may give to SSIZE_MAX.  So
> 
>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal
> default, use it.
>  (3) all other cases, us our internal default.


oops, yes, of course

/* allow overwriting from e.g. Makefile */
#ifndef(MAX_IO_SIZE)
# define MAX_IO_SIZE (8*1024*1024)
  /* for plattforms that have SSIZE and have it smaller */
# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)
#  undef MAX_IO_SIZE /* avoid warning */
#  define MAX_IO_SIZE SSIZE_MAX
# endif
#endif

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Eric Sunshine]

On Sun, Feb 8, 2015 at 7:05 AM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
> Junio C Hamano <gitster <at> pobox.com> writes:
>>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.
>>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal
>> default, use it.
>>  (3) all other cases, us our internal default.
>
> oops, yes, of course
>
> /* allow overwriting from e.g. Makefile */
> #ifndef(MAX_IO_SIZE)
> # define MAX_IO_SIZE (8*1024*1024)
>   /* for plattforms that have SSIZE and have it smaller */
> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)
> #  undef MAX_IO_SIZE /* avoid warning */
> #  define MAX_IO_SIZE SSIZE_MAX
> # endif
> #endif

A bit cleaner:

#ifndef(MAX_IO_SIZE)
# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
#  define MAX_IO_SIZE SSIZE_MAX
# else
#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
# endif
#endif

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> A bit cleaner:
>
> #ifndef(MAX_IO_SIZE)
> # define MAX_IO_SIZE_DEFAULT (8*1024*1024)
> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
> #  define MAX_IO_SIZE SSIZE_MAX
> # else
> #  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
> # endif
> #endif

OK, then let's do this.

-- >8 --
Subject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX

Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20),
we chomp our calls to read(2) and write(2) into chunks of
MAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad
latency when the program needs to be killed.  This also brought our
IO below SSIZE_MAX, which is a limit POSIX allows read(2) and
write(2) to fail when the IO size exceeds it, for OS X, where a
problem was originally reported.

However, there are other systems that define SSIZE_MAX smaller than
our default X-<.  Make sure we clip our calls to this as well.

Reported-by: Joachim Schmitz <jojo@schmitz-digital.de>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wrapper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/wrapper.c b/wrapper.c
index 007ec0d..50e6697 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -172,8 +172,21 @@ void *xcalloc(size_t nmemb, size_t size)
  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
  * the absence of bugs, large chunks can result in bad latencies when
  * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than taht is allowed to fail.  As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX platform is broken.
  */
-#define MAX_IO_SIZE (8*1024*1024)
+#ifndef(MAX_IO_SIZE)
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+#  define MAX_IO_SIZE SSIZE_MAX
+# else
+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
 
 /*
  * xread() is the same a read(), but it automatically restarts read()

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Junio C Hamano <gitster <at> pobox.com> writes:

<snip> 
> OK, then let's do this.
> 
Yep, that'd do, thanks.

bye, Jojo

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Joachim Schmitz <jojo <at> schmitz-digital.de> writes:

> 
> Junio C Hamano <gitster <at> pobox.com> writes:
> 
> <snip> 
> > OK, then let's do this.
> > 


Except for the type "taht"

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Junio C Hamano]

Joachim Schmitz <jojo@schmitz-digital.de> writes:

> Joachim Schmitz <jojo <at> schmitz-digital.de> writes:
>
>> 
>> Junio C Hamano <gitster <at> pobox.com> writes:
>> 
>> <snip> 
>> > OK, then let's do this.
>> > 
>
>
> Except for the type "taht"

Also #ifndef part X-<

Here is what I queued for the day.

-- >8 --
Subject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX

Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20),
we chomp our calls to read(2) and write(2) into chunks of
MAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad
latency when the program needs to be killed.  This also brought our
IO below SSIZE_MAX, which is a limit POSIX allows read(2) and
write(2) to fail when the IO size exceeds it, for OS X, where a
problem was originally reported.

However, there are other systems that define SSIZE_MAX smaller than
our default, and feeding 8 MiB to underlying read(2)/write(2) would
fail.  Make sure we clip our calls to the lower limit as well.

Reported-by: Joachim Schmitz <jojo@schmitz-digital.de>
Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wrapper.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/wrapper.c b/wrapper.c
index f92b147..c77c2eb 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -135,8 +135,21 @@ void *xcalloc(size_t nmemb, size_t size)
  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in
  * the absense of bugs, large chunks can result in bad latencies when
  * you decide to kill the process.
+ *
+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX
+ * that is smaller than that, clip it to SSIZE_MAX, as a call to
+ * read(2) or write(2) larger than that is allowed to fail.  As the last
+ * resort, we allow a port to pass via CFLAGS e.g. "-DMAX_IO_SIZE=value"
+ * to override this, if the definition of SSIZE_MAX platform is broken.
  */
-#define MAX_IO_SIZE (8*1024*1024)
+#ifndef MAX_IO_SIZE
+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)
+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)
+#  define MAX_IO_SIZE SSIZE_MAX
+# else
+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT
+# endif
+#endif
 
 /*
  * xread() is the same a read(), but it automatically restarts read()
-- 
2.3.0-186-g9f73ee1

Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?

[Joachim Schmitz]

Sorry to be a pain, but i think this sententence neede mending

+ * to override this, if the definition of SSIZE_MAX platform is broken.

Bye, Jojo