writev is "unfair".

[Sebastian Wilhelmi]

Hi,

As one of the co-developers of ORBit I recently ran into the following probem:

writev first tries to just call the kernel's writev and if that fails due to a
oversized array of iovec's, it calls a fallback writev implementation, which
in turn allocates the necessary memory on the stack (alloca), copies the
contents of the iovec's there and "write"s it out.

Now some user of ORBit wanted to transmit real big buffers of data and thus he
ended up with a core dump, because of stack overrun (I suspect) in the
fallback writev. What I am proposing now is to change the fallback
implementation sysdeps/posix/writev.c to alloca at most (let's say) 10KB and
write bigger buffers directly. There of course could be problems with error
reporting. I do not fully know, how to handle that 100% correctly, but I have
a preliminary patch attached to solve that. It is against CVS libc, but not
fully tested, as CVS libc does not compile on my system (name-config.h
missing, once that is fixed sysdeps/unix/i386/fork.S refuses to compile, maybe
because of gcc-2.95.2)

I just wanted to know, if such a change would at least have the chance to be
included, or if it isn't possible because of e.g. diagnosis semantics, that
require to call write inside writev exactly once.

Bye,
Sebastian
-- 
Sebastian Wilhelmi
mailto:address@hidden
http://goethe.ira.uka.de/~wilhelmi

Index: sysdeps/posix/writev.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/posix/writev.c,v
retrieving revision 1.8
diff -u -b -B -r1.8 writev.c
--- writev.c    1997/11/05 23:59:21     1.8
+++ writev.c    2000/11/27 13:48:54
@@ -16,6 +16,7 @@
    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
+#include <errno.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
@@ -26,29 +27,54 @@
    The data is written in the order specified.
    Operates just like `write' (see <unistd.h>) except that the data
    are taken from VECTOR instead of a contiguous buffer.  */
+
+#define MAX_ALLOCA_SIZE 10 * 1024 /* 10 KB */
+
 ssize_t
 __writev (fd, vector, count)
      int fd;
      const struct iovec *vector;
      int count;
 {
-  char *buffer;
-  register char *bp;
-  size_t bytes, to_copy;
-  int i;
+  int round_start = 0;
+  int round_end;
+  char *buffer = NULL; 
+  size_t bytes_written = 0;
 
+  while (round_start < count)
+    {
+      int i;
+      char *write_pos;
+      size_t write_len = 0;
+      ssize_t result;
   /* Find the total number of bytes to be written.  */
-  bytes = 0;
-  for (i = 0; i < count; ++i)
-    bytes += vector[i].iov_len;
-
-  /* Allocate a temporary buffer to hold the data.  */
-  buffer = (char *) __alloca (bytes);
+      for (round_end = round_start; 
+          round_end < count && write_len < MAX_ALLOCA_SIZE; ++i)
+       write_len += vector[round_end].iov_len;
 
+      /* There is only one iov_base to write */
+      if (round_end == round_start || round_end == round_start + 1)
+       {
+         write_len = vector[round_start].iov_len;
+         round_end = round_start + 1;
+         write_pos = vector[round_start].iov_base;
+       }
+      else
+       {
+         size_t to_copy;
+         register char *bp;
+         if (!buffer)
+           {
+             /* If we need more than one loop, we allocate maximal
+              * size, otherwise 'write_len' is enough */
+             size_t to_alloc = 
+               (round_end != count) ? MAX_ALLOCA_SIZE : write_len;
+             buffer = (char *) __alloca (to_alloc);
+           }
   /* Copy the data into BUFFER.  */
-  to_copy = bytes;
+         to_copy = write_len;
   bp = buffer;
-  for (i = 0; i < count; ++i)
+         for (i = round_start; i < round_end; ++i)
     {
 #define        min(a, b)       ((a) > (b) ? (b) : (a))
       size_t copy = min (vector[i].iov_len, to_copy);
@@ -59,8 +85,26 @@
       if (to_copy == 0)
        break;
     }
+         write_pos = buffer;
+       }
 
-  return __write (fd, buffer, bytes);
+      result = __write (fd, write_pos, write_len);
+      if (result != write_len)
+       {
+         if (result != -1)
+           return bytes_written + result;
+         else if (bytes_written == 0)
+           return -1;
+         else if (errno == EAGAIN || errno == EINTR)
+           return bytes_written;
+         else
+           return -1;
+       }
+      
+      bytes_written += result;
+      round_start = round_end;
+    }
+  return bytes_written;
 }
 #ifndef __writev
 weak_alias (__writev, writev)