Attachment 47950 Details for Bug 38288 – work-in-progress patch which adds DBusWriteVector

[patch] work-in-progress patch which adds DBusWriteVector

0001-WiP.patch (text/plain), 16.76 KB, created by Simon McVittie on 2011-06-14 03:58:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-06-14 03:58:52 UTC

Size: 16.76 KB

patch

obsolete

>From d2640d6040787634d7d989080ffba2225312229d Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Fri, 10 Jun 2011 13:42:26 +0100
>Subject: [PATCH] WiP
>
>---
> dbus/dbus-iovec.h          |  130 ++++++++++++++++++++++++
> dbus/dbus-marshal-header.c |    5 +-
> dbus/dbus-marshal-header.h |    8 ++-
> dbus/dbus-sysdeps-unix.c   |  234 ++++++++++++++------------------------------
> dbus/dbus-sysdeps.h        |   21 +---
> 5 files changed, 219 insertions(+), 179 deletions(-)
> create mode 100644 dbus/dbus-iovec.h
>
>diff --git a/dbus/dbus-iovec.h b/dbus/dbus-iovec.h
>new file mode 100644
>index 0000000..4f56bcc
>--- /dev/null
>+++ b/dbus/dbus-iovec.h
>@@ -0,0 +1,130 @@
>+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
>+/* dbus-iovec.h - writev() emulation
>+ *
>+ * Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
>+ * Copyright Â© 2011 Nokia Corporation
>+ *
>+ * Licensed under the Academic Free License version 2.1
>+ *
>+ * This program is free software; you can redistribute it and/or modify
>+ * it under the terms of the GNU General Public License as published by
>+ * the Free Software Foundation; either version 2 of the License, or
>+ * (at your option) any later version.
>+ *
>+ * This program is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>+ * GNU General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU General Public License
>+ * along with this program; if not, write to the Free Software
>+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
>+ *
>+ */
>+
>+#ifndef DBUS_IOVEC_H
>+#define DBUS_IOVEC_H
>+
>+#include "dbus/dbus-internals.h"
>+#include "dbus/dbus-string.h"
>+
>+DBUS_BEGIN_DECLS
>+
>+typedef struct DBusWriteVector DBusWriteVector;
>+
>+struct DBusWriteVector {
>+    /** number of items to write */
>+    int n_items;
>+    /** start next write from:
>+     * strings[skip_items][offsets[skip_items] + skip_bytes] */
>+    int skip_items;
>+    int skip_bytes;
>+
>+    /** i'th write: strings[i][offsets[i] .. offsets[i] + lengths[i]] */
>+    /** array of DBusString, length n_items */
>+    const DBusString * const *strings;
>+    /** array of offset into strings[i], length n_items */
>+    const int *offsets;
>+    /** array of length of substring of strings[i], length n_items */
>+    const int *lengths;
>+};
>+
>+static inline dbus_bool_t
>+_dbus_write_vector_progress (DBusWriteVector *vec,
>+                             int wrote_bytes)
>+{
>+  while (wrote_bytes > 0)
>+    {
>+      int left;
>+
>+      if (vec->skip_items >= vec->n_items)
>+        return FALSE;
>+
>+      left = lengths[vec->skip_items] - vec->skip_bytes;
>+
>+      _dbus_assert (left > 0);
>+
>+      if (left > wrote_bytes)
>+        {
>+          /* there's more to write in the current item */
>+          vec->skip_bytes += wrote_bytes;
>+          return TRUE;
>+        }
>+      else
>+        {
>+          /* we've written all the bytes of this item, move to the next */
>+          wrote_bytes -= left;
>+          vec->skip_bytes = 0;
>+          vec->skip_items += 1;
>+        }
>+    }
>+}
>+
>+/**
>+ * @param vec a write-vector
>+ * @param n the buffer to write (0 if using write(), start from 0 and iterate
>+ *  if using writev())
>+ * @param str return the nth string to write part of
>+ * @param offset return the index into *str to start writing
>+ * @param length return the length of the part of *str to write
>+ * @returns #TRUE, setting str, offset and length, if at least n+1 buffers
>+ *  remain to be written; #FALSE if not
>+ */
>+static inline dbus_bool_t
>+_dbus_write_vector_get_nth (DBusWriteVector *vec,
>+                            unsigned int n,
>+                            const DBusString **str,
>+                            int *offset,
>+                            int *length)
>+{
>+  /* skip over any zero-length items just after the "cursor" */
>+  if (!_dbus_write_vector_progress (vec, 0))
>+    return FALSE;
>+
>+  if (n == 0)
>+    {
>+      /* first item can be partial */
>+      *str = strings[vec->skip_items];
>+      *offset = offsets[vec->skip_items] + vec->skip_bytes;
>+      *length = lengths[vec->skip_items] - vec->skip_bytes;
>+      _dbus_assert (*length > 0);
>+      return TRUE;
>+    }
>+
>+  do
>+    {
>+      if (vec->skip_items + n >= vec->n_items)
>+        return FALSE;
>+
>+      *str = strings[vec->skip_items + n];
>+      *offset = offsets[vec->skip_items + n];
>+      *length = offsets[vec->skip_items + n];
>+    }
>+  while (*length == 0);
>+
>+  return TRUE;
>+}
>+
>+DBUS_END_DECLS
>+
>+#endif
>diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c
>index 715d953..91af2e9 100644
>--- a/dbus/dbus-marshal-header.c
>+++ b/dbus/dbus-marshal-header.c
>@@ -22,8 +22,11 @@
>  */
> 
> #include <config.h>
>-#include "dbus/dbus-shared.h"
>+
>+#define _DBUS_IN_MARSHAL_HEADER_C
> #include "dbus-marshal-header.h"
>+
>+#include "dbus/dbus-shared.h"
> #include "dbus-marshal-recursive.h"
> #include "dbus-marshal-byteswap.h"
> 
>diff --git a/dbus/dbus-marshal-header.h b/dbus/dbus-marshal-header.h
>index 350fe5c..c18f434 100644
>--- a/dbus/dbus-marshal-header.h
>+++ b/dbus/dbus-marshal-header.h
>@@ -33,6 +33,12 @@ typedef struct DBusHeaderField DBusHeaderField;
> #define _DBUS_HEADER_FIELD_VALUE_UNKNOWN -1
> #define _DBUS_HEADER_FIELD_VALUE_NONEXISTENT -2
> 
>+#ifdef _DBUS_IN_MARSHAL_HEADER_C
>+#define _DBUS_PRIV(x) x
>+#else
>+#define _DBUS_PRIV(x) _priv_ ## x
>+#endif
>+
> /**
>  * Cached information about a header field in the message
>  */
>@@ -46,7 +52,7 @@ struct DBusHeaderField
>  */
> struct DBusHeader
> {
>-  DBusString data; /**< Header network data, stored
>+  DBusString _DBUS_PRIV (data); /**< Header network data, stored
>                     * separately from body so we can
>                     * independently realloc it.
>                     */
>diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
>index e1e1728..8b9d96a 100644
>--- a/dbus/dbus-sysdeps-unix.c
>+++ b/dbus/dbus-sysdeps-unix.c
>@@ -365,31 +365,24 @@ _dbus_write_socket_with_unix_fds(int               fd,
>                                  int               start,
>                                  int               len,
>                                  const int        *fds,
>-                                 int               n_fds) {
>+                                 int               n_fds)
>+{
>+  DBusWriteVector vec = { 0 };
> 
>-#ifndef HAVE_UNIX_FD_PASSING
>+  vec->n_items = 1;
>+  vec->strings = &buffer;
>+  vec->offsets = &start;
>+  vec->lengths = &len;
> 
>-  if (n_fds > 0) {
>-    errno = ENOTSUP;
>-    return -1;
>-  }
>-
>-  return _dbus_write_socket(fd, buffer, start, len);
>-#else
>-  return _dbus_write_socket_with_unix_fds_two(fd, buffer, start, len, NULL, 0, 0, fds, n_fds);
>-#endif
>+  return _dbus_write_socket_with_unix_fds_vec (fd, &vec, fds, n_fds);
> }
> 
> int
>-_dbus_write_socket_with_unix_fds_two(int               fd,
>-                                     const DBusString *buffer1,
>-                                     int               start1,
>-                                     int               len1,
>-                                     const DBusString *buffer2,
>-                                     int               start2,
>-                                     int               len2,
>-                                     const int        *fds,
>-                                     int               n_fds) {
>+_dbus_write_socket_with_unix_fds_vec (int               fd,
>+                                      DBusWriteVector  *vec,
>+                                      const int        *fds,
>+                                      int               n_fds)
>+{
> 
> #ifndef HAVE_UNIX_FD_PASSING
> 
>@@ -398,9 +391,9 @@ _dbus_write_socket_with_unix_fds_two(int               fd,
>     return -1;
>   }
> 
>-  return _dbus_write_socket_two(fd,
>-                                buffer1, start1, len1,
>-                                buffer2, start2, len2);
>+  return _dbus_write_socket_vec (fd,
>+                                 buffer1, start1, len1,
>+                                 buffer2, start2, len2);
> #else
> 
>   struct msghdr m;
>@@ -459,76 +452,6 @@ _dbus_write_socket_with_unix_fds_two(int               fd,
> #endif
> }
> 
>-/**
>- * Like _dbus_write_two() but only works on sockets and is thus
>- * available on Windows.
>- *
>- * @param fd the file descriptor
>- * @param buffer1 first buffer
>- * @param start1 first byte to write in first buffer
>- * @param len1 number of bytes to write from first buffer
>- * @param buffer2 second buffer, or #NULL
>- * @param start2 first byte to write in second buffer
>- * @param len2 number of bytes to write in second buffer
>- * @returns total bytes written from both buffers, or -1 on error
>- */
>-int
>-_dbus_write_socket_two (int               fd,
>-                        const DBusString *buffer1,
>-                        int               start1,
>-                        int               len1,
>-                        const DBusString *buffer2,
>-                        int               start2,
>-                        int               len2)
>-{
>-#if HAVE_DECL_MSG_NOSIGNAL
>-  struct iovec vectors[2];
>-  const char *data1;
>-  const char *data2;
>-  int bytes_written;
>-  struct msghdr m;
>-
>-  _dbus_assert (buffer1 != NULL);
>-  _dbus_assert (start1 >= 0);
>-  _dbus_assert (start2 >= 0);
>-  _dbus_assert (len1 >= 0);
>-  _dbus_assert (len2 >= 0);
>-
>-  data1 = _dbus_string_get_const_data_len (buffer1, start1, len1);
>-
>-  if (buffer2 != NULL)
>-    data2 = _dbus_string_get_const_data_len (buffer2, start2, len2);
>-  else
>-    {
>-      data2 = NULL;
>-      start2 = 0;
>-      len2 = 0;
>-    }
>-
>-  vectors[0].iov_base = (char*) data1;
>-  vectors[0].iov_len = len1;
>-  vectors[1].iov_base = (char*) data2;
>-  vectors[1].iov_len = len2;
>-
>-  _DBUS_ZERO(m);
>-  m.msg_iov = vectors;
>-  m.msg_iovlen = data2 ? 2 : 1;
>-
>- again:
>-
>-  bytes_written = sendmsg (fd, &m, MSG_NOSIGNAL);
>-
>-  if (bytes_written < 0 && errno == EINTR)
>-    goto again;
>-
>-  return bytes_written;
>-
>-#else
>-  return _dbus_write_two (fd, buffer1, start1, len1,
>-                          buffer2, start2, len2);
>-#endif
>-}
>-
> dbus_bool_t
> _dbus_socket_is_invalid (int fd)
> {
>@@ -637,92 +560,81 @@ _dbus_write (int               fd,
>   return bytes_written;
> }
> 
>+/* SUS mandates that IOVEC_LEN (the maximum number of iovec structs to write
>+ * at once) is at least 63 */
>+#define DBUS_IOVEC_LEN (63)
>+
> /**
>- * Like _dbus_write() but will use writev() if possible
>- * to write both buffers in sequence. The return value
>- * is the number of bytes written in the first buffer,
>- * plus the number written in the second. If the first
>- * buffer is written successfully and an error occurs
>- * writing the second, the number of bytes in the first
>- * is returned (i.e. the error is ignored), on systems that
>- * don't have writev. Handles EINTR for you.
>- * The second buffer may be #NULL.
>+ * Similar to _dbus_write_socket(), but will use writev() or sendmsg() if
>+ * possible to write all buffers in sequence. Handles EINTR for you.
>  *
>  * @param fd the file descriptor
>- * @param buffer1 first buffer
>- * @param start1 first byte to write in first buffer
>- * @param len1 number of bytes to write from first buffer
>- * @param buffer2 second buffer, or #NULL
>- * @param start2 first byte to write in second buffer
>- * @param len2 number of bytes to write in second buffer
>- * @returns total bytes written from both buffers, or -1 on error
>+ * @param vec vector of buffers to write
>+ * @returns 1 if more writing is needed, 0 if writing has finished, negative
>+ *  with errno set on error
>  */
> int
>-_dbus_write_two (int               fd,
>-                 const DBusString *buffer1,
>-                 int               start1,
>-                 int               len1,
>-                 const DBusString *buffer2,
>-                 int               start2,
>-                 int               len2)
>-{
>-  _dbus_assert (buffer1 != NULL);
>-  _dbus_assert (start1 >= 0);
>-  _dbus_assert (start2 >= 0);
>-  _dbus_assert (len1 >= 0);
>-  _dbus_assert (len2 >= 0);
>+_dbus_write_socket_vec (int               fd,
>+                        DBusWriteVector  *vec)
>+{
>+  const DBusString *str;
>+  int offset, length;
>+  int bytes_written;
> 
>-#ifdef HAVE_WRITEV
>+#if HAVE_DECL_MSG_NOSIGNAL || defined (HAVE_WRITEV)
>   {
>-    struct iovec vectors[2];
>-    const char *data1;
>-    const char *data2;
>-    int bytes_written;
>-
>-    data1 = _dbus_string_get_const_data_len (buffer1, start1, len1);
>+    struct iovec vectors[DBUS_IOVEC_LEN];
>+    unsigned int n;
>+#ifdef HAVE_DECL_MSG_NOSIGNAL
>+    struct msgheader m;
>+#endif
> 
>-    if (buffer2 != NULL)
>-      data2 = _dbus_string_get_const_data_len (buffer2, start2, len2);
>-    else
>+    for (n = 0;
>+        n < DBUS_IOVEC_LEN &&
>+          _dbus_write_vector_get_nth (vec, n, &str, &offset, &len);
>+        n++)
>       {
>-        data2 = NULL;
>-        start2 = 0;
>-        len2 = 0;
>+        vectors[n].iov_base = (char *)
>+          _dbus_string_get_const_data_len (str, offset, len);
>+        vectors[n].iov_len = len;
>       }
> 
>-    vectors[0].iov_base = (char*) data1;
>-    vectors[0].iov_len = len1;
>-    vectors[1].iov_base = (char*) data2;
>-    vectors[1].iov_len = len2;
>+    /* all written? */
>+    if (n == 0)
>+      return 0;
> 
>-  again:
>+#ifdef HAVE_DECL_MSG_NOSIGNAL
>+    _DBUS_ZERO (m);
>+    m.msg_iov = vectors;
>+    m.msg_iovlen = n;
>+#endif
> 
>-    bytes_written = writev (fd,
>-                            vectors,
>-                            data2 ? 2 : 1);
>+    do
>+      {
>+#ifdef HAVE_DECL_MSG_NOSIGNAL
>+        bytes_written = sendmsg (fd, &m, MSG_NOSIGNAL);
>+#else
>+        bytes_written = writev (fd, vectors, n);
>+#endif
>+      }
>+    while (bytes_written < 0 && errno == EINTR);
>+  }
>+#else /* !HAVE_DECL_MSG_NO_SIGNAL && !defined (HAVE_WRITEV) */
> 
>-    if (bytes_written < 0 && errno == EINTR)
>-      goto again;
>+  if (!_dbus_write_vector_get_nth (vec, 0, &str, &offset, &len))
>+    return FALSE;
> 
>-    return bytes_written;
>-  }
>-#else /* HAVE_WRITEV */
>-  {
>-    int ret1;
>+  bytes_written = _dbus_write (fd, str, offset, len);
> 
>-    ret1 = _dbus_write (fd, buffer1, start1, len1);
>-    if (ret1 == len1 && buffer2 != NULL)
>-      {
>-        ret2 = _dbus_write (fd, buffer2, start2, len2);
>-        if (ret2 < 0)
>-          ret2 = 0; /* we can't report an error as the first write was OK */
>+#endif /* !HAVE_DECL_MSG_NO_SIGNAL && !defined (HAVE_WRITEV) */
> 
>-        return ret1 + ret2;
>-      }
>-    else
>-      return ret1;
>-  }
>-#endif /* !HAVE_WRITEV */
>+  if (bytes_written < 0)
>+    return -1;
>+
>+  if (_dbus_write_vector_progress (vec, bytes_written))
>+    return 1;
>+
>+  return 0;
> }
> 
> #define _DBUS_MAX_SUN_PATH_LENGTH 99
>diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
>index 22d7969..007b43d 100644
>--- a/dbus/dbus-sysdeps.h
>+++ b/dbus/dbus-sysdeps.h
>@@ -33,6 +33,7 @@
> 
> #include <dbus/dbus-errors.h>
> #include <dbus/dbus-file.h>
>+#include <dbus/dbus-iovec.h>
> #include <dbus/dbus-string.h>
> 
> /* this is perhaps bogus, but strcmp() etc. are faster if we use the
>@@ -132,13 +133,8 @@ int         _dbus_write_socket     (int               fd,
>                                     const DBusString *buffer,
>                                     int               start,
>                                     int               len);
>-int         _dbus_write_socket_two (int               fd,
>-                                    const DBusString *buffer1,
>-                                    int               start1,
>-                                    int               len1,
>-                                    const DBusString *buffer2,
>-                                    int               start2,
>-                                    int               len2);
>+int         _dbus_write_socket_vec (int               fd,
>+                                    DBusWriteVector  *vec);
> 
> int _dbus_read_socket_with_unix_fds      (int               fd,
>                                           DBusString       *buffer,
>@@ -151,15 +147,8 @@ int _dbus_write_socket_with_unix_fds     (int               fd,
>                                           int               len,
>                                           const int        *fds,
>                                           int               n_fds);
>-int _dbus_write_socket_with_unix_fds_two (int               fd,
>-                                          const DBusString *buffer1,
>-                                          int               start1,
>-                                          int               len1,
>-                                          const DBusString *buffer2,
>-                                          int               start2,
>-                                          int               len2,
>-                                          const int        *fds,
>-                                          int               n_fds);
>+int _dbus_write_socket_with_unix_fds_vec (int               fd,
>+                                          DBusWriteVector  *vec);
> 
> dbus_bool_t _dbus_socket_is_invalid (int              fd);
> 
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 38288: 47950

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.