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

2015-02-11 Thread Junio C Hamano
Eric Sunshine sunsh...@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 j...@schmitz-digital.de
Helped-by: Torsten Bögershausen tbo...@web.de
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Junio C Hamano gits...@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()
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-11 Thread Joachim Schmitz
Junio C Hamano gitster at pobox.com writes:

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

bye, Jojo



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-11 Thread 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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-11 Thread Junio C Hamano
Joachim Schmitz j...@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 j...@schmitz-digital.de
Helped-by: Torsten Bögershausen tbo...@web.de
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Junio C Hamano gits...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-11 Thread 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

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

2015-02-08 Thread 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







--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-08 Thread Eric Sunshine
On Sun, Feb 8, 2015 at 7:05 AM, Joachim Schmitz j...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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?

2015-02-07 Thread 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.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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?

2015-02-07 Thread Joachim Schmitz
Joachim Schmitz jojo at schmitz-digital.de writes:

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

bye, Jojo


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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 ?

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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?

2015-02-07 Thread 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



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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 ?



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread 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, 
JojoN�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf

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

2015-02-07 Thread 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 j...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread Junio C Hamano
On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen tbo...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-02-07 Thread Junio C Hamano
On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz j...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html