Re: write() _will_ fail on Mac OS X/XNU if nbytes > INT_MAX

Filipe Cabecinhas [off-list ref] writes:

Due to a bug in the Darwin kernel, write() calls have a maximum size of
INT_MAX bytes.

This patch introduces a new compat function: clipped_write
This function behaves the same as write() but will write, at most, INT_MAX
characters.
It may be necessary to include this function on Windows, too.

Signed-off-by: Filipe Cabecinhas <redacted>
---

Somehow your MUA seems to have lost _all_ tabs, not just in the new
lines in your patch but also in the existing context lines.

Here is what I resurrected and queued. I _think_ I did not make any
silly mistake while transcribing from your whitespace-mangled patch,
but please double check; I do not have any Darwin, so this hasn't
even been compile tested.

Also I have a small suggestion I'd like you to try on top of it,
which I'll be sending in a separate message.

Thanks.

-- >8 --
From: Filipe Cabecinhas <redacted>
Date: Fri, 10 May 2013 15:24:57 -0700
Subject: [PATCH] compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Due to a bug in the Darwin kernel, write(2) calls have a maximum size
of INT_MAX bytes.

Introduce a new compat function, clipped_write(), that only writes
at most INT_MAX bytes and returns the number of bytes written, as
a substitute for write(2), and allow platforms that need this to
enable it from the build mechanism with NEEDS_CLIPPED_WRITE.

Set it for Mac OS X by default.  It may be necessary to include this
function on Windows, too.

Signed-off-by: Filipe Cabecinhas <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 Makefile               |  5 +++++
 compat/clipped-write.c | 13 +++++++++++++
 config.mak.uname       |  1 +
 git-compat-util.h      |  5 +++++
 4 files changed, 24 insertions(+)
 create mode 100644 compat/clipped-write.c

diff --git a/Makefile b/Makefile
index 26d3332..7076b15 100644
--- a/Makefile
+++ b/Makefile

@@ -1463,6 +1463,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
 	MSGFMT += --check --statistics
 endif
 
+ifdef NEEDS_CLIPPED_WRITE
+	BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
+	COMPAT_OBJS += compat/clipped-write.o
+endif
+
 ifneq (,$(XDL_FAST_HASH))
 	BASIC_CFLAGS += -DXDL_FAST_HASH
 endif

diff --git a/compat/clipped-write.c b/compat/clipped-write.c
new file mode 100644
index 0000000..9183698
--- /dev/null
+++ b/compat/clipped-write.c

@@ -0,0 +1,13 @@
+#include <limits.h>
+#include <unistd.h>
+
+/*
+ * 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)
+		nbyte = INT_MAX;
+	return write(fildes, buf, nbyte);
+}

diff --git a/config.mak.uname b/config.mak.uname
index e09af8f..e689a9a 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 b636e0d..3144b8d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -181,6 +181,11 @@ typedef unsigned long uintptr_t;
 #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);

-- 
1.8.3-rc1-268-g30389da

Hi Junio,

Thanks for helping. Your text is correct and only diffs from my patch
in the #define write(...) part, where I suppose you stripped the
spaced in the arglist.

Thank you,

  Filipe

  F


On Fri, May 10, 2013 at 4:05 PM, Junio C Hamano [off-list ref] wrote:

Hi Junio,

It compiles cleanly and runs. I'm running the test suite anyway, but
don't expect any change from your latest patch.

Thank you,

  Filipe
  F


On Fri, May 10, 2013 at 4:13 PM, Filipe Cabecinhas [off-list ref] wrote:

Hi Junio,

Thanks for helping. Your text is correct and only diffs from my patch
in the #define write(...) part, where I suppose you stripped the
spaced in the arglist.

Thank you,

  Filipe

  F


On Fri, May 10, 2013 at 4:05 PM, Junio C Hamano [off-list ref] wrote:

quoted

Filipe Cabecinhas [off-list ref] writes:

quoted

Due to a bug in the Darwin kernel, write() calls have a maximum size of
INT_MAX bytes.

This patch introduces a new compat function: clipped_write
This function behaves the same as write() but will write, at most, INT_MAX
characters.
It may be necessary to include this function on Windows, too.

Signed-off-by: Filipe Cabecinhas <redacted>
---

Somehow your MUA seems to have lost _all_ tabs, not just in the new
lines in your patch but also in the existing context lines.

quoted

diff --git a/Makefile b/Makefile
index 0f931a2..ccb8f3f 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/clipped-write.o
+endif
+

...

Here is what I resurrected and queued. I _think_ I did not make any
silly mistake while transcribing from your whitespace-mangled patch,
but please double check; I do not have any Darwin, so this hasn't
even been compile tested.

Also I have a small suggestion I'd like you to try on top of it,
which I'll be sending in a separate message.

Thanks.

-- >8 --
From: Filipe Cabecinhas <redacted>
Date: Fri, 10 May 2013 15:24:57 -0700
Subject: [PATCH] compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Due to a bug in the Darwin kernel, write(2) calls have a maximum size
of INT_MAX bytes.

Introduce a new compat function, clipped_write(), that only writes
at most INT_MAX bytes and returns the number of bytes written, as
a substitute for write(2), and allow platforms that need this to
enable it from the build mechanism with NEEDS_CLIPPED_WRITE.

Set it for Mac OS X by default.  It may be necessary to include this
function on Windows, too.

Signed-off-by: Filipe Cabecinhas <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 Makefile               |  5 +++++
 compat/clipped-write.c | 13 +++++++++++++
 config.mak.uname       |  1 +
 git-compat-util.h      |  5 +++++
 4 files changed, 24 insertions(+)
 create mode 100644 compat/clipped-write.c

diff --git a/Makefile b/Makefile
index 26d3332..7076b15 100644
--- a/Makefile
+++ b/Makefile

@@ -1463,6 +1463,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
        MSGFMT += --check --statistics
 endif

+ifdef NEEDS_CLIPPED_WRITE
+       BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
+       COMPAT_OBJS += compat/clipped-write.o
+endif
+
 ifneq (,$(XDL_FAST_HASH))
        BASIC_CFLAGS += -DXDL_FAST_HASH
 endif

diff --git a/compat/clipped-write.c b/compat/clipped-write.c
new file mode 100644
index 0000000..9183698
--- /dev/null
+++ b/compat/clipped-write.c

@@ -0,0 +1,13 @@
+#include <limits.h>
+#include <unistd.h>
+
+/*
+ * 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)
+               nbyte = INT_MAX;
+       return write(fildes, buf, nbyte);
+}

diff --git a/config.mak.uname b/config.mak.uname
index e09af8f..e689a9a 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 b636e0d..3144b8d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -181,6 +181,11 @@ typedef unsigned long uintptr_t;
 #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);
--

1.8.3-rc1-268-g30389da

I know I'm extremely late to the party, and this patch has already
landed, but...

On Sat, May 11, 2013 at 1:05 AM, Junio C Hamano [off-list ref] wrote:

Filipe Cabecinhas [off-list ref] writes:

quoted

Due to a bug in the Darwin kernel, write() calls have a maximum size of
INT_MAX bytes.

This patch introduces a new compat function: clipped_write
This function behaves the same as write() but will write, at most, INT_MAX
characters.
It may be necessary to include this function on Windows, too.

We are already doing something similar for Windows in mingw_write (see
compat/mingw.c), but with a much smaller size.

It feels a bit pointless to duplicate this logic.

If we were to reuse this logic with Windows, we'd need to have some
way of overriding the max-size of the write.

Hej,
I think the patch went in and out in git.git, please see below.

(I coudn't the following  in msysgit,
 but if it was there, the clipped_write() for Windows could go away.

/Torsten



commit 0b6806b9e45c659d25b87fb5713c920a3081eac8
Author: Steffen Prohaska [off-list ref]
Date:   Tue Aug 20 08:43:54 2013 +0200

    xread, xwrite: limit size of IO to 8MB
    
    Checking out 2GB or more through an external filter (see test) fails
    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 filter cat failed 141
        error: external filter cat failed
    
    The reason is that read() immediately returns with EINVAL when asked
    to read more than 2GB.  According to POSIX [1], if the value of
    nbyte passed to read() is greater than SSIZE_MAX, the result is
    implementation-defined.  The write function has the same restriction
    [2].  Since OS X still supports running 32-bit executables, the
    32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also
    imposed on 64-bit executables under certain conditions.  For write,
    the problem has been addressed earlier [6c642a].
    
    Address the problem for read() and write() differently, by limiting
    size of IO chunks unconditionally on all platforms in xread() and
    xwrite().  Large chunks only cause problems, like causing latencies
    when killing the process, even if OS X was not buggy.  Doing IO in
    reasonably sized smaller chunks should have no negative impact on
    performance.
    
    The compat wrapper clipped_write() introduced earlier [6c642a] is
    not needed anymore.  It will be reverted in a separate commit.  The
    new test catches read and write problems.
    
    Note that 'git add' exits with 0 even if it prints filtering errors
    to stderr.  The test, therefore, checks stderr.  'git add' should
    probably be changed (sometime in another commit) to exit with
    nonzero if filtering fails.  The test could then be changed to use
    test_must_fail.


On 2013-11-20 11.15, Erik Faye-Lund wrote:

I know I'm extremely late to the party, and this patch has already
landed, but...

On Sat, May 11, 2013 at 1:05 AM, Junio C Hamano [off-list ref] wrote:

quoted

Filipe Cabecinhas [off-list ref] writes:

quoted

Due to a bug in the Darwin kernel, write() calls have a maximum size of
INT_MAX bytes.

This patch introduces a new compat function: clipped_write
This function behaves the same as write() but will write, at most, INT_MAX
characters.
It may be necessary to include this function on Windows, too.

We are already doing something similar for Windows in mingw_write (see
compat/mingw.c), but with a much smaller size.

It feels a bit pointless to duplicate this logic.

quoted

diff --git a/compat/clipped-write.c b/compat/clipped-write.c
new file mode 100644
index 0000000..9183698
--- /dev/null
+++ b/compat/clipped-write.c

@@ -0,0 +1,13 @@
+#include <limits.h>
+#include <unistd.h>
+
+/*
+ * 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)
+               nbyte = INT_MAX;
+       return write(fildes, buf, nbyte);
+}

If we were to reuse this logic with Windows, we'd need to have some
way of overriding the max-size of the write.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.