{"thread":{"id":"38523","subject":"read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","startedAt":"2015-02-07T16:45:39Z","lastAt":"2015-02-11T23:15:47Z","messageCount":20,"participants":["Joachim Schmitz","Torsten Bögershausen","Randall S. Becker","Junio C Hamano","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"255696","messageId":"loom.20150207T174514-727@post.gmane.org","threadId":"38523","inReplyTo":null,"subject":"read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T16:45:39Z","receivedAt":"2015-02-07T16:45:39Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Hi there\n\nWhile investigating the problem with hung git-upload-pack we think to have \nfound a bug in wrapper.c:\n\n#define MAX_IO_SIZE (8*1024*1024)\n\nThis is then used in xread() to split read()s into suitable chunks.\nSo far so good, but read() is only guaranteed to read as much as SSIZE_MAX \nbytes at a time. And on our platform that is way lower than those 8MB (only \n52kB, POSIX allows it to be as small as 32k), and as a (rather strange) \nconsequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), \nbecause xpread() returns something > 0.\n\nHow large is SSIZE_MAX on other platforms? What happens there if you try to \nread() more? Should't we rather use SSIZE_MAX on all platforms? If I'm \nreading the header files right, on Linux it is LONG_MAX (2TB?), so I guess \nwe should really go for MIN(8*1024*1024,SSIZE_MAX)?\n\n\nbye, Jojo\n"},{"id":"255697","messageId":"loom.20150207T174757-863@post.gmane.org","threadId":"38523","inReplyTo":"loom.20150207T174514-727@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T16:48:54Z","receivedAt":"2015-02-07T16:48:54Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Joachim Schmitz <jojo <at> schmitz-digital.de> writes:\n\n> because xpread() returns something > 0.\n something < 0 of course (presumably -1)...\n\nbye, Jojo\n"},{"id":"255699","messageId":"54D64939.4080102@web.de","threadId":"38523","inReplyTo":"loom.20150207T174514-727@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-07T17:19:53Z","receivedAt":"2015-02-07T17:19:53Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-02-07 17.45, Joachim Schmitz wrote:\n> Hi there\n> \n> While investigating the problem with hung git-upload-pack we think to have \n> found a bug in wrapper.c:\n> \n> #define MAX_IO_SIZE (8*1024*1024)\n> \n> This is then used in xread() to split read()s into suitable chunks.\n> So far so good, but read() is only guaranteed to read as much as SSIZE_MAX \n> bytes at a time. And on our platform that is way lower than those 8MB (only \n> 52kB, POSIX allows it to be as small as 32k), and as a (rather strange) \n> consequence mmap() (from compat/mmap.c) fails with EACCESS (why EACCESS?), \n> because xpread() returns something > 0.\n> \n> How large is SSIZE_MAX on other platforms? What happens there if you try to \n> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm \n> reading the header files right, on Linux it is LONG_MAX (2TB?), so I guess \n> we should really go for MIN(8*1024*1024,SSIZE_MAX)?\n\nHow about changing wrapper.c like this:\n\n#ifndef MAX_IO_SIZE\n #define MAX_IO_SIZE (8*1024*1024)\n#endif\n---------------------\nand to change config.mak.uname like this:\n\nifeq ($(uname_S),NONSTOP_KERNEL)\n\n\tBASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)\nDoes this work for you ?\n"},{"id":"255700","messageId":"loom.20150207T182443-33@post.gmane.org","threadId":"38523","inReplyTo":"54D64939.4080102@web.de","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T17:29:17Z","receivedAt":"2015-02-07T17:29:17Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Torsten Bögershausen <tboegi <at> web.de> writes:\n\n> \n> On 2015-02-07 17.45, Joachim Schmitz wrote:\n<snip>\n> \n> How about changing wrapper.c like this:\n> \n> #ifndef MAX_IO_SIZE\n>  #define MAX_IO_SIZE (8*1024*1024)\n> #endif\n> ---------------------\n> and to change config.mak.uname like this:\n> \n> ifeq ($(uname_S),NONSTOP_KERNEL)\n> \n> \tBASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)\n> Does this work for you ?\n\nOf course it would, but, \na) 32k is smaller than we can go (and yes, we could make it 52k)\nb) never ever should read() be asked to read more than SSIZE_MAX, this  \nshould be true for every platform on the planet? You may want to have is \nsmaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on \nLinux), but surely never larger?\n\nBye, Jojo"},{"id":"255701","messageId":"loom.20150207T185751-446@post.gmane.org","threadId":"38523","inReplyTo":"loom.20150207T182443-33@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T18:03:37Z","receivedAt":"2015-02-07T18:03:37Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Joachim Schmitz <jojo <at> schmitz-digital.de> writes:\n\n> \n> Torsten Bögershausen <tboegi <at> web.de> writes:\n> \n> > \n> > On 2015-02-07 17.45, Joachim Schmitz wrote:\n<snip>\n> b) never ever should read() be asked to read more than SSIZE_MAX, this  \n> should be true for every platform on the planet? You may want to have is \n> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on \n> Linux), but surely never larger?\n\nSe also $gmane/232469, where that issue cropped up for MacOS X 64bit?\n\nBye, Jojo\n\n\n"},{"id":"255702","messageId":"01f201d04300$cce22ca0$66a685e0$@nexbridge.com","threadId":"38523","inReplyTo":"54D64939.4080102@web.de","subject":"RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-02-07T18:06:30Z","receivedAt":"2015-02-07T18:06:30Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On 2015-02-07 12:30PM Torsten Bögershausen wrote:\n>On 2015-02-07 17.45, Joachim Schmitz wrote:\n>> Hi there\n>> \n>> While investigating the problem with hung git-upload-pack we think to \n>> have found a bug in wrapper.c:\n>> \n>> #define MAX_IO_SIZE (8*1024*1024)\n>> \n>> This is then used in xread() to split read()s into suitable chunks.\n>> So far so good, but read() is only guaranteed to read as much as \n>> SSIZE_MAX bytes at a time. And on our platform that is way lower than \n>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a \n>> (rather strange) consequence mmap() (from compat/mmap.c) fails with \n>> EACCESS (why EACCESS?), because xpread() returns something > 0.\n>> \n>> How large is SSIZE_MAX on other platforms? What happens there if you \n>> try to\n>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm \n>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I \n>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?\n>How about changing wrapper.c like this: \n>#ifndef MAX_IO_SIZE\n> #define MAX_IO_SIZE (8*1024*1024)\n>#endif\n>---------------------\n>and to change config.mak.uname like this:\n>ifeq ($(uname_S),NONSTOP_KERNEL)\n>\tBASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024) Does this work for you ?\n\nYes, thank you Torsten. I have made this change in our branch (on behalf of\nJojo). I think we can accept it. The (32*1024) does need to be properly\nquoted, however.\n"},{"id":"255703","messageId":"01f501d04302$bb9a46b0$32ced410$@nexbridge.com","threadId":"38523","inReplyTo":"01f201d04300$cce22ca0$66a685e0$@nexbridge.com","subject":"RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-02-07T18:20:20Z","receivedAt":"2015-02-07T18:20:20Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On 2015-02-07 13:07PM Randall S. Becker wrote:\n>On 2015-02-07 12:30PM Torsten Bögershausen wrote:\n>>On 2015-02-07 17.45, Joachim Schmitz wrote:\n>>> Hi there\n>>> \n>>> While investigating the problem with hung git-upload-pack we think to \n>>> have found a bug in wrapper.c:\n>>> \n>>> #define MAX_IO_SIZE (8*1024*1024)\n>>> \n>>> This is then used in xread() to split read()s into suitable chunks.\n>>> So far so good, but read() is only guaranteed to read as much as \n>>> SSIZE_MAX bytes at a time. And on our platform that is way lower than \n>>> those 8MB (only 52kB, POSIX allows it to be as small as 32k), and as a \n>>> (rather strange) consequence mmap() (from compat/mmap.c) fails with \n>>> EACCESS (why EACCESS?), because xpread() returns something > 0.\n>>> \n>>> How large is SSIZE_MAX on other platforms? What happens there if you \n>>> try to\n>>> read() more? Should't we rather use SSIZE_MAX on all platforms? If I'm \n>>> reading the header files right, on Linux it is LONG_MAX (2TB?), so I \n>>> guess we should really go for MIN(8*1024*1024,SSIZE_MAX)?\n>>How about changing wrapper.c like this: \n>>#ifndef MAX_IO_SIZE\n>> #define MAX_IO_SIZE (8*1024*1024)\n>>#endif\n>>---------------------\n\nAlthough I do agree with Jojo, that MAX_IO_SIZE seems to be a platform\nconstant and should be defined in terms of SSIZE_MAX. So something like:\n\n#ifndef MAX_IO_SIZE\n# ifdef SSIZE_MAX\n#  define MAX_IO_SIZE (SSIZE_MAX)\n# else\n#  define MAX_IO_SIZE (8*1024*1024)\n# endif\n#endif\n\nwould be desirable.\n\nCheers, Randall\n"},{"id":"255704","messageId":"loom.20150207T193242-470@post.gmane.org","threadId":"38523","inReplyTo":"01f501d04302$bb9a46b0$32ced410$@nexbridge.com","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T18:36:35Z","receivedAt":"2015-02-07T18:36:35Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Randall S. Becker <rsbecker <at> nexbridge.com> writes:\n\n> \n> On 2015-02-07 13:07PM Randall S. Becker wrote:\n> >On 2015-02-07 12:30PM Torsten Bögershausen wrote:\n> >>On 2015-02-07 17.45, Joachim Schmitz wrote:\n<spip> \n> Although I do agree with Jojo, that MAX_IO_SIZE seems to be a platform\n> constant and should be defined in terms of SSIZE_MAX. So something like:\n> \n> #ifndef MAX_IO_SIZE\n> # ifdef SSIZE_MAX\n> #  define MAX_IO_SIZE (SSIZE_MAX)\n> # else\n> #  define MAX_IO_SIZE (8*1024*1024)\n> # endif\n> #endif\n> \n> would be desirable.\n\nIt would be way too large on some platforms. those 8MB had been chosen for \na good reason, I assume:\n\n/*\n * Limit size of IO chunks, because huge chunks only cause pain.  OS X\n * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in\n * the absence of bugs, large chunks can result in bad latencies when\n * you decide to kill the process.\n */\n\nHowever it should never be larger than SSIZE_MAX\n\n"},{"id":"255705","messageId":"loom.20150207T201128-590@post.gmane.org","threadId":"38523","inReplyTo":"loom.20150207T174514-727@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T19:14:06Z","receivedAt":"2015-02-07T19:14:06Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Joachim Schmitz <jojo <at> schmitz-digital.de> writes:\n\n<snip>\n> and as a (rather strange) \n> consequence mmap() (from compat/mmap.c) fails with EACCESS (why \nEACCESS?), \n> because xpread() returns something > 0.\n\nSeems mmap() should either set errno to EINVAL or not set it at all an \njust 'forward' whatever xpread() has set.\n\nAs per http://man7.org/linux/man-pages/man2/mmap.2.html mmap() sets EINVAL \nif (amongst other things) it doesn't like the value of len, exactly the \ncase here.\n\nbye, Jojo\n"},{"id":"255708","messageId":"54D67662.7040504@web.de","threadId":"38523","inReplyTo":"loom.20150207T182443-33@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-07T20:32:34Z","receivedAt":"2015-02-07T20:32:34Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-02-07 18.29, Joachim Schmitz wrote:\n> Torsten Bögershausen <tboegi <at> web.de> writes:\n> \n>>\n>> On 2015-02-07 17.45, Joachim Schmitz wrote:\n> <snip>\n>>\n>> How about changing wrapper.c like this:\n>>\n>> #ifndef MAX_IO_SIZE\n>>  #define MAX_IO_SIZE (8*1024*1024)\n>> #endif\n>> ---------------------\n>> and to change config.mak.uname like this:\n>>\n>> ifeq ($(uname_S),NONSTOP_KERNEL)\n>>\n>> \tBASIC_CFLAGS += -DMAX_IO_SIZE=(32*1024)\n>> Does this work for you ?\n> \n> Of course it would, but, \n> a) 32k is smaller than we can go (and yes, we could make it 52k)\nSorry, I missed that:  (52*1024)\n> b) never ever should read() be asked to read more than SSIZE_MAX, this  \n> should be true for every platform on the planet? You may want to have is \n> smaller than SSIZE_MAX (like the current 8MB vs. the possible 2TB on \n> Linux), but surely never larger?\n> \nGood question.\nI don't know every platform of the planet well enough to be helpful here,\nespecially the ones which don't follow all the specifications.\n\nIn other words: As long as we can not guarantee that SSIZE_MAX is defined,\n(and is defined to somethong useful for xread()/xwrite() )\nwe should be more defensive here:\n\ntweak only on platform where we know it is needed and we know that it works.\n\nAnd leave the other ones alone, until someone finds another\nplatform which needs the same or another tweak and sends a tested patch.\n\n\nThanks for the report, do you want to send a patch to the list ?\n"},{"id":"255710","messageId":"CAPc5daUnKcktv0xcz-fGEApckbkQksKuZO53ZL20E1MhtZmn4w@mail.gmail.com","threadId":"38523","inReplyTo":"54D67662.7040504@web.de","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-07T22:13:29Z","receivedAt":"2015-02-07T22:13:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi@web.de> wrote:\n> I don't know every platform of the planet well enough to be helpful here,\n> especially the ones which don't follow all the specifications.\n>\n> In other words: As long as we can not guarantee that SSIZE_MAX is defined,\n> (and is defined to somethong useful for xread()/xwrite() )\n> we should be more defensive here:\n>\n> tweak only on platform where we know it is needed and we know that it works.\n\nYup, I agree that is a sensible way to go.\n\n (1) if Makefile overrides the size, use it; otherwise\n (2) if SSIZE_MAX is defined, and it is smaller than our internal\ndefault, use it; otherwise\n (3) use our internal default.\n\nAnd leave our internal default to 8MB.\n\nThat way, nobody needs to do anything differently from his current build set-up,\nand I suspect that it would make step (1) optional.\n"},{"id":"255712","messageId":"loom.20150207T232422-706@post.gmane.org","threadId":"38523","inReplyTo":"CAPc5daUnKcktv0xcz-fGEApckbkQksKuZO53ZL20E1MhtZmn4w@mail.gmail.com","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-07T22:31:39Z","receivedAt":"2015-02-07T22:31:39Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> \n> On Sat, Feb 7, 2015 at 12:32 PM, Torsten Bögershausen <tboegi <at> \nweb.de> wrote:\n> > I don't know every platform of the planet well enough to be helpful \nhere,\n> > especially the ones which don't follow all the specifications.\n> >\n> > In other words: As long as we can not guarantee that SSIZE_MAX is \ndefined,\n> > (and is defined to somethong useful for xread()/xwrite() )\n> > we should be more defensive here:\n> >\n> > tweak only on platform where we know it is needed and we know that it \nworks.\n> \n> Yup, I agree that is a sensible way to go.\n> \n>  (1) if Makefile overrides the size, use it; otherwise\n>  (2) if SSIZE_MAX is defined, and it is smaller than our internal\n> default, use it; otherwise\n>  (3) use our internal default.\n> \n> And leave our internal default to 8MB.\n> \n> That way, nobody needs to do anything differently from his current build \nset-up,\n> and I suspect that it would make step (1) optional.\n> \n\nsomething like this:\n\n/* allow overwriting from e.g. Makefile */\n#if !defined(MAX_IO_SIZE)\n# define MAX_IO_SIZE (8*1024*1024)\n#endif\n/* for plattforms that have SSIZE and have it smaller */\n#if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) \n# undef MAX_IO_SIZE /* avoid warning */\n# define MAX_IO_SIZE SSIZE_MAX\n#endif\n\nSteps 2 and 3 only , indeed step 1 not needed...\n\nBye, Jojo"},{"id":"255713","messageId":"CAPc5daXD_7XZD5Vag51BjrSZ0q1r9eMswhLmnpUFqqjrc9oSTw@mail.gmail.com","threadId":"38523","inReplyTo":"loom.20150207T232422-706@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-08T02:13:47Z","receivedAt":"2015-02-08T02:13:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:\n> Junio C Hamano <gitster <at> pobox.com> writes:\n>>\n>> Yup, I agree that is a sensible way to go.\n>>\n>>  (1) if Makefile overrides the size, use it; otherwise\n>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal\n>> default, use it; otherwise\n>>  (3) use our internal default.\n>>\n>> And leave our internal default to 8MB.\n>>\n>> That way, nobody needs to do anything differently from his current build\n> set-up,\n>> and I suspect that it would make step (1) optional.\n>\n> something like this:\n>\n> /* allow overwriting from e.g. Makefile */\n> #if !defined(MAX_IO_SIZE)\n> # define MAX_IO_SIZE (8*1024*1024)\n> #endif\n> /* for plattforms that have SSIZE and have it smaller */\n> #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)\n> # undef MAX_IO_SIZE /* avoid warning */\n> # define MAX_IO_SIZE SSIZE_MAX\n> #endif\n\nNo, not like that. If you do (1), that is only so that the Makefile can override\na broken definition a platform may give to SSIZE_MAX.  So\n\n (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.\n (2) if SSIZE_MAX is defined, and if it is smaller than our internal\ndefault, use it.\n (3) all other cases, us our internal default.\n"},{"id":"255714","messageId":"020e01d04347$7efbe200$7cf3a600$@nexbridge.com","threadId":"38523","inReplyTo":"CAPc5daXD_7XZD5Vag51BjrSZ0q1r9eMswhLmnpUFqqjrc9oSTw@mail.gmail.com","subject":"RE: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2015-02-08T02:32:33Z","receivedAt":"2015-02-08T02:32:33Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Feb 7 2015 at 9:14 PM Junio C Hamano wrote:\n>On Sat, Feb 7, 2015 at 2:31 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:\n>> Junio C Hamano <gitster <at> pobox.com> writes:\n>>>\n>>> Yup, I agree that is a sensible way to go.\n>>>\n>>>  (1) if Makefile overrides the size, use it; otherwise\n>>>  (2) if SSIZE_MAX is defined, and it is smaller than our internal \n>>> default, use it; otherwise\n>>>  (3) use our internal default.\n>>>\n>>> And leave our internal default to 8MB.\n>>>\n>>> That way, nobody needs to do anything differently from his current \n>>> build\n>> set-up,\n>>> and I suspect that it would make step (1) optional.\n>>\n>> something like this:\n>>\n>> /* allow overwriting from e.g. Makefile */ #if !defined(MAX_IO_SIZE) # \n>> define MAX_IO_SIZE (8*1024*1024) #endif\n>> /* for plattforms that have SSIZE and have it smaller */ #if \n>> defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE) # undef MAX_IO_SIZE /* \n>> avoid warning */ # define MAX_IO_SIZE SSIZE_MAX #endif\n>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\n> (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.\n> (2) if SSIZE_MAX is defined, and if it is smaller than our internal default, use it.\n> (3) all other cases, us our internal default.\n\nThat is reasonable. I am more concerned about our git-upload-pak (separate thread) anyway :)\n\nCheers, Randall\n"},{"id":"255762","messageId":"loom.20150208T125055-287@post.gmane.org","threadId":"38523","inReplyTo":"CAPc5daXD_7XZD5Vag51BjrSZ0q1r9eMswhLmnpUFqqjrc9oSTw@mail.gmail.com","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-08T12:05:34Z","receivedAt":"2015-02-08T12:05:34Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n<snip>\n> >\n> > something like this:\n> >\n> > /* allow overwriting from e.g. Makefile */\n> > #if !defined(MAX_IO_SIZE)\n> > # define MAX_IO_SIZE (8*1024*1024)\n> > #endif\n> > /* for plattforms that have SSIZE and have it smaller */\n> > #if defined(SSIZE_MAX && (SSIZE_MAX < MAX_IO_SIZE)\n> > # undef MAX_IO_SIZE /* avoid warning */\n> > # define MAX_IO_SIZE SSIZE_MAX\n> > #endif\n> \n> No, not like that. If you do (1), that is only so that the Makefile can \noverride\n> a broken definition a platform may give to SSIZE_MAX.  So\n> \n>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.\n>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal\n> default, use it.\n>  (3) all other cases, us our internal default.\n\n\noops, yes, of course\n\n/* allow overwriting from e.g. Makefile */\n#ifndef(MAX_IO_SIZE)\n# define MAX_IO_SIZE (8*1024*1024)\n  /* for plattforms that have SSIZE and have it smaller */\n# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)\n#  undef MAX_IO_SIZE /* avoid warning */\n#  define MAX_IO_SIZE SSIZE_MAX\n# endif\n#endif\n"},{"id":"255781","messageId":"CAPig+cTbLOV-0yFKp8wwVLSr4OJz7LUaLZgVGHUdFhj7xZEzrw@mail.gmail.com","threadId":"38523","inReplyTo":"loom.20150208T125055-287@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-02-08T17:09:20Z","receivedAt":"2015-02-08T17:09:20Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 8, 2015 at 7:05 AM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:\n> Junio C Hamano <gitster <at> pobox.com> writes:\n>>  (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX.\n>>  (2) if SSIZE_MAX is defined, and if it is smaller than our internal\n>> default, use it.\n>>  (3) all other cases, us our internal default.\n>\n> oops, yes, of course\n>\n> /* allow overwriting from e.g. Makefile */\n> #ifndef(MAX_IO_SIZE)\n> # define MAX_IO_SIZE (8*1024*1024)\n>   /* for plattforms that have SSIZE and have it smaller */\n> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE)\n> #  undef MAX_IO_SIZE /* avoid warning */\n> #  define MAX_IO_SIZE SSIZE_MAX\n> # endif\n> #endif\n\nA bit cleaner:\n\n#ifndef(MAX_IO_SIZE)\n# define MAX_IO_SIZE_DEFAULT (8*1024*1024)\n# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)\n#  define MAX_IO_SIZE SSIZE_MAX\n# else\n#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT\n# endif\n#endif\n"},{"id":"255943","messageId":"xmqqmw4ktamx.fsf@gitster.dls.corp.google.com","threadId":"38523","inReplyTo":"CAPig+cTbLOV-0yFKp8wwVLSr4OJz7LUaLZgVGHUdFhj7xZEzrw@mail.gmail.com","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-11T21:13:10Z","receivedAt":"2015-02-11T21:13:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> A bit cleaner:\n>\n> #ifndef(MAX_IO_SIZE)\n> # define MAX_IO_SIZE_DEFAULT (8*1024*1024)\n> # if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)\n> #  define MAX_IO_SIZE SSIZE_MAX\n> # else\n> #  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT\n> # endif\n> #endif\n\nOK, then let's do this.\n\n-- >8 --\nSubject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX\n\nSince 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20),\nwe chomp our calls to read(2) and write(2) into chunks of\nMAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad\nlatency when the program needs to be killed.  This also brought our\nIO below SSIZE_MAX, which is a limit POSIX allows read(2) and\nwrite(2) to fail when the IO size exceeds it, for OS X, where a\nproblem was originally reported.\n\nHowever, there are other systems that define SSIZE_MAX smaller than\nour default X-<.  Make sure we clip our calls to this as well.\n\nReported-by: Joachim Schmitz <jojo@schmitz-digital.de>\nHelped-by: Torsten Bögershausen <tboegi@web.de>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n wrapper.c | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex 007ec0d..50e6697 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -172,8 +172,21 @@ void *xcalloc(size_t nmemb, size_t size)\n  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in\n  * the absence of bugs, large chunks can result in bad latencies when\n  * you decide to kill the process.\n+ *\n+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX\n+ * that is smaller than that, clip it to SSIZE_MAX, as a call to\n+ * read(2) or write(2) larger than taht is allowed to fail.  As the last\n+ * resort, we allow a port to pass via CFLAGS e.g. \"-DMAX_IO_SIZE=value\"\n+ * to override this, if the definition of SSIZE_MAX platform is broken.\n  */\n-#define MAX_IO_SIZE (8*1024*1024)\n+#ifndef(MAX_IO_SIZE)\n+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)\n+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)\n+#  define MAX_IO_SIZE SSIZE_MAX\n+# else\n+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT\n+# endif\n+#endif\n \n /*\n  * xread() is the same a read(), but it automatically restarts read()\n"},{"id":"255946","messageId":"loom.20150211T222833-105@post.gmane.org","threadId":"38523","inReplyTo":"xmqqmw4ktamx.fsf@gitster.dls.corp.google.com","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-11T21:29:18Z","receivedAt":"2015-02-11T21:29:18Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n<snip> \n> OK, then let's do this.\n> \nYep, that'd do, thanks.\n\nbye, Jojo\n"},{"id":"255948","messageId":"loom.20150211T230519-703@post.gmane.org","threadId":"38523","inReplyTo":"loom.20150211T222833-105@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2015-02-11T22:05:50Z","receivedAt":"2015-02-11T22:05:50Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Joachim Schmitz <jojo <at> schmitz-digital.de> writes:\n\n> \n> Junio C Hamano <gitster <at> pobox.com> writes:\n> \n> <snip> \n> > OK, then let's do this.\n> > \n\n\nExcept for the type \"taht\"\n"},{"id":"255951","messageId":"xmqq1tlwt4yk.fsf@gitster.dls.corp.google.com","threadId":"38523","inReplyTo":"loom.20150211T230519-703@post.gmane.org","subject":"Re: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-11T23:15:47Z","receivedAt":"2015-02-11T23:15:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joachim Schmitz <jojo@schmitz-digital.de> writes:\n\n> Joachim Schmitz <jojo <at> schmitz-digital.de> writes:\n>\n>> \n>> Junio C Hamano <gitster <at> pobox.com> writes:\n>> \n>> <snip> \n>> > OK, then let's do this.\n>> > \n>\n>\n> Except for the type \"taht\"\n\nAlso #ifndef part X-<\n\nHere is what I queued for the day.\n\n-- >8 --\nSubject: xread/xwrite: clip MAX_IO_SIZE to SSIZE_MAX\n\nSince 0b6806b9 (xread, xwrite: limit size of IO to 8MB, 2013-08-20),\nwe chomp our calls to read(2) and write(2) into chunks of\nMAX_IO_SIZE bytes (8 MiB), because a large IO results in a bad\nlatency when the program needs to be killed.  This also brought our\nIO below SSIZE_MAX, which is a limit POSIX allows read(2) and\nwrite(2) to fail when the IO size exceeds it, for OS X, where a\nproblem was originally reported.\n\nHowever, there are other systems that define SSIZE_MAX smaller than\nour default, and feeding 8 MiB to underlying read(2)/write(2) would\nfail.  Make sure we clip our calls to the lower limit as well.\n\nReported-by: Joachim Schmitz <jojo@schmitz-digital.de>\nHelped-by: Torsten Bögershausen <tboegi@web.de>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n wrapper.c | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex f92b147..c77c2eb 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -135,8 +135,21 @@ void *xcalloc(size_t nmemb, size_t size)\n  * 64-bit is buggy, returning EINVAL if len >= INT_MAX; and even in\n  * the absense of bugs, large chunks can result in bad latencies when\n  * you decide to kill the process.\n+ *\n+ * We pick 8 MiB as our default, but if the platform defines SSIZE_MAX\n+ * that is smaller than that, clip it to SSIZE_MAX, as a call to\n+ * read(2) or write(2) larger than that is allowed to fail.  As the last\n+ * resort, we allow a port to pass via CFLAGS e.g. \"-DMAX_IO_SIZE=value\"\n+ * to override this, if the definition of SSIZE_MAX platform is broken.\n  */\n-#define MAX_IO_SIZE (8*1024*1024)\n+#ifndef MAX_IO_SIZE\n+# define MAX_IO_SIZE_DEFAULT (8*1024*1024)\n+# if defined(SSIZE_MAX) && (SSIZE_MAX < MAX_IO_SIZE_DEFAULT)\n+#  define MAX_IO_SIZE SSIZE_MAX\n+# else\n+#  define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT\n+# endif\n+#endif\n \n /*\n  * xread() is the same a read(), but it automatically restarts read()\n-- \n2.3.0-186-g9f73ee1\n"}]}