From: Filipe Cabecinhas Date: Thu, 09 May 2013 22:58:25 GMT Subject: Re: write() _will_ fail on Mac OS X/XNU if nbytes > INT_MAX Message-ID: In-Reply-To: <7v4nffpbct.fsf@alter.siamese.dyndns.org> Hi Junio, Sorry for the delay. I've updated the patch to work as you suggested (I think). It's attached. Thank you, Filipe F On Tue, Apr 9, 2013 at 3:50 PM, Junio C Hamano wrote: > Filipe Cabecinhas writes: > >> >> Testing with dd bs=INT_MAX+1 count=1 also gets me an “Invalid >> argument” error, while bs=INT_MAX will do what's expected. >> >> I have a preliminary patch that fixes it, but it may not be the >> preferred way. The code is not ifdef'ed out and I'm doing the fix in >> write_in_full(), while it may be preferred to do the fix in xwrite(). >> >> A radar bug has been submitted to Apple about this, but I think git >> could tolerate the bug while it isn't fixed, by working around it. >> >> Thank you, >> >> Filipe >> >> diff --git a/wrapper.c b/wrapper.c >> index bac59d2..474d760 100644 >> --- a/wrapper.c >> +++ b/wrapper.c >> @@ -187,7 +187,12 @@ ssize_t write_in_full(int fd, const void *buf, size_t count) >> ssize_t total = 0; >> >> while (count > 0) { >> - ssize_t written = xwrite(fd, p, count); >> + ssize_t written = 0; >> + if (count >= INT_MAX) >> + written = xwrite(fd, p, INT_MAX-1); >> + else >> + written = xwrite(fd, p, count); > > I think it is better to fix it in much lower level of the stack, as > other codepaths would call write(2), either thru xwrite() or > directly. > > Ideally the fix should go to the lowest level, i.e. the write(2). I > do not care if it is done in the kernel or in the libc wrapping > code; the above does not belong to our code (in an ideal world, that > is). > > Otherwise you would have to patch everything in /usr/bin, no? > > But you do not live in an ideal world and neither do we. I think > the least ugly way may be to add compat/clipped-write.c that > implements a loop like the above in a helper function: > > ssize_t clipped_write(int, const void *, size_t); > > and have a > > #ifdef NEED_CLIPPED_WRITE > #define write(x,y,z) clipped_write((x),(y),(z)) > #endif > > in git-compat-util.h, or something. Makefile needs to get adjusted > to link with compat/clipped-write.o when NEED_CLIPPED_WRITE is > defined as well. diff --git a/Makefile b/Makefile index 0f931a2..e6aff85 100644 --- a/Makefile +++ b/Makefile @@ -1466,6 +1466,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS MSGFMT += --check --statistics endif +ifdef NEEDS_CLIPPED_WRITE + BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE + COMPAT_OBJS += compat/write.o +endif + ifneq (,$(XDL_FAST_HASH)) BASIC_CFLAGS += -DXDL_FAST_HASH endif diff --git a/compat/write.c b/compat/write.c new file mode 100644 index 0000000..1e890aa --- /dev/null +++ b/compat/write.c @@ -0,0 +1,11 @@ +#include +#include + +/* Version of write that will write at most INT_MAX bytes. + * Workaround a xnu bug on Mac OS X */ +ssize_t clipped_write(int fildes, const void *buf, size_t nbyte) { + if (nbyte > INT_MAX) + return write(fildes, buf, INT_MAX); + else + return write(fildes, buf, nbyte); +} diff --git a/config.mak.uname b/config.mak.uname index d78fd3d..174703b 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin) NO_MEMMEM = YesPlease USE_ST_TIMESPEC = YesPlease HAVE_DEV_TTY = YesPlease + NEEDS_CLIPPED_WRITE = YesPlease COMPAT_OBJS += compat/precompose_utf8.o BASIC_CFLAGS += -DPRECOMPOSE_UNICODE endif diff --git a/git-compat-util.h b/git-compat-util.h index e955bb5..a96db23 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -185,6 +185,11 @@ int get_st_mode_bits(const char *path, int *mode); #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NEEDS_CLIPPED_WRITE +ssize_t clipped_write(int fildes, const void *buf, size_t nbyte); +#define write(x, y, z) clipped_write((x), (y), (z)) +#endif + #ifdef MKDIR_WO_TRAILING_SLASH #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b)) extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);