Attachment 47948 Details for Bug 38287 – [PATCH 1/2] DBusMessage: don't redundantly store byte order, ask the DBusHeader

[patch] [PATCH 1/2] DBusMessage: don't redundantly store byte order, ask the DBusHeader

0001-DBusMessage-don-t-redundantly-store-byte-order-ask-t.patch (text/plain), 10.09 KB, created by Simon McVittie on 2011-06-14 03:12:21 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-06-14 03:12:21 UTC

Size: 10.09 KB

patch

obsolete

>From 973ab3b1e89d9c22a47d94491794edd6f983a217 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Fri, 10 Jun 2011 10:10:54 +0100
>Subject: [PATCH 1/2] DBusMessage: don't redundantly store byte order, ask the
> DBusHeader
>
>---
> dbus/dbus-marshal-header.c  |   26 +++++++++++++++++++-------
> dbus/dbus-marshal-header.h  |    1 +
> dbus/dbus-message-factory.c |   14 ++++++++++----
> dbus/dbus-message-private.h |    2 --
> dbus/dbus-message.c         |   37 +++++++++++++++++++------------------
> 5 files changed, 49 insertions(+), 31 deletions(-)
>
>diff --git a/dbus/dbus-marshal-header.c b/dbus/dbus-marshal-header.c
>index a6c9b80..2fdf558 100644
>--- a/dbus/dbus-marshal-header.c
>+++ b/dbus/dbus-marshal-header.c
>@@ -165,6 +165,20 @@ _dbus_header_cache_one (DBusHeader     *header,
> }
> 
> /**
>+ * Returns the header's byte order.
>+ *
>+ * @param header the header
>+ * @returns the byte order
>+ */
>+char
>+_dbus_header_get_byte_order (const DBusHeader *header)
>+{
>+  _dbus_assert (_dbus_string_get_length (&header->data) > BYTE_ORDER_OFFSET);
>+
>+  return (char) _dbus_string_get_byte (&header->data, BYTE_ORDER_OFFSET);
>+}
>+
>+/**
>  * Revalidates the fields cache
>  *
>  * @param header the header
>@@ -1468,21 +1482,19 @@ void
> _dbus_header_byteswap (DBusHeader *header,
>                        int         new_order)
> {
>-  unsigned char byte_order;
>+  char byte_order;
> 
>-  if (header->byte_order == new_order)
>-    return;
>+  byte_order = _dbus_header_get_byte_order (header);
> 
>-  byte_order = _dbus_string_get_byte (&header->data, BYTE_ORDER_OFFSET);
>-  _dbus_assert (header->byte_order == byte_order);
>+  if (byte_order == new_order)
>+    return;
> 
>   _dbus_marshal_byteswap (&_dbus_header_signature_str,
>-                          0, header->byte_order,
>+                          0, byte_order,
>                           new_order,
>                           &header->data, 0);
> 
>   _dbus_string_set_byte (&header->data, BYTE_ORDER_OFFSET, new_order);
>-  header->byte_order = new_order;
> }
> 
> /** @} */
>diff --git a/dbus/dbus-marshal-header.h b/dbus/dbus-marshal-header.h
>index fd16c5f..8713de3 100644
>--- a/dbus/dbus-marshal-header.h
>+++ b/dbus/dbus-marshal-header.h
>@@ -122,6 +122,7 @@ dbus_bool_t   _dbus_header_load                   (DBusHeader        *header,
>                                                    int                len);
> void          _dbus_header_byteswap               (DBusHeader        *header,
>                                                    int                new_order);
>+char          _dbus_header_get_byte_order         (const DBusHeader  *header);
> 
> 
> 
>diff --git a/dbus/dbus-message-factory.c b/dbus/dbus-message-factory.c
>index 7fae583..b5a06f1 100644
>--- a/dbus/dbus-message-factory.c
>+++ b/dbus/dbus-message-factory.c
>@@ -170,6 +170,7 @@ generate_many_bodies_inner (DBusMessageDataIter *iter,
>   DBusMessage *message;
>   DBusString signature;
>   DBusString body;
>+  char byte_order;
> 
>   /* Keeping this small makes things go faster */
>   message = dbus_message_new_method_call ("o.z.F",
>@@ -179,13 +180,15 @@ generate_many_bodies_inner (DBusMessageDataIter *iter,
>   if (message == NULL)
>     _dbus_assert_not_reached ("oom");
> 
>+  byte_order = _dbus_header_get_byte_order (&message->header);
>+
>   set_reply_serial (message);
> 
>   if (!_dbus_string_init (&signature) || !_dbus_string_init (&body))
>     _dbus_assert_not_reached ("oom");
>   
>   if (dbus_internal_do_not_use_generate_bodies (iter_get_sequence (iter),
>-                                                message->byte_order,
>+                                                byte_order,
>                                                 &signature, &body))
>     {
>       const char *v_SIGNATURE;
>@@ -202,7 +205,7 @@ generate_many_bodies_inner (DBusMessageDataIter *iter,
> 
>       _dbus_marshal_set_uint32 (&message->header.data, BODY_LENGTH_OFFSET,
>                                 _dbus_string_get_length (&message->body),
>-                                message->byte_order);
>+                                byte_order);
>       
>       *message_p = message;
>     }
>@@ -578,15 +581,18 @@ generate_special (DBusMessageDataIter   *iter,
>     }
>   else if (item_seq == 8)
>     {
>+      char byte_order;
>+
>       message = simple_method_call ();
>+      byte_order = _dbus_header_get_byte_order (&message->header);
>       generate_from_message (data, expected_validity, message);
>       
>       _dbus_marshal_set_uint32 (data, BODY_LENGTH_OFFSET,
>                                 DBUS_MAXIMUM_MESSAGE_LENGTH / 2 + 4,
>-                                message->byte_order);
>+                                byte_order);
>       _dbus_marshal_set_uint32 (data, FIELDS_ARRAY_LENGTH_OFFSET,
>                                 DBUS_MAXIMUM_MESSAGE_LENGTH / 2 + 4,
>-                                message->byte_order);
>+                                byte_order);
>       *expected_validity = DBUS_INVALID_MESSAGE_TOO_LONG;
>     }
>   else if (item_seq == 9)
>diff --git a/dbus/dbus-message-private.h b/dbus/dbus-message-private.h
>index c5e3b3e..e64258c 100644
>--- a/dbus/dbus-message-private.h
>+++ b/dbus/dbus-message-private.h
>@@ -102,8 +102,6 @@ struct DBusMessage
> 
>   DBusString body;   /**< Body network data. */
> 
>-  char byte_order; /**< Message byte order. */
>-
>   unsigned int locked : 1; /**< Message being sent, no modifications allowed. */
> 
> #ifndef DBUS_DISABLE_CHECKS
>diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
>index a59ed9b..b208f52 100644
>--- a/dbus/dbus-message.c
>+++ b/dbus/dbus-message.c
>@@ -115,8 +115,11 @@ _dbus_message_byteswap (DBusMessage *message)
> {
>   const DBusString *type_str;
>   int type_pos;
>-  
>-  if (message->byte_order == DBUS_COMPILER_BYTE_ORDER)
>+  char byte_order;
>+
>+  byte_order = _dbus_header_get_byte_order (&message->header);
>+
>+  if (byte_order == DBUS_COMPILER_BYTE_ORDER)
>     return;
> 
>   _dbus_verbose ("Swapping message into compiler byte order\n");
>@@ -124,13 +127,13 @@ _dbus_message_byteswap (DBusMessage *message)
>   get_const_signature (&message->header, &type_str, &type_pos);
>   
>   _dbus_marshal_byteswap (type_str, type_pos,
>-                          message->byte_order,
>+                          byte_order,
>                           DBUS_COMPILER_BYTE_ORDER,
>                           &message->body, 0);
> 
>-  message->byte_order = DBUS_COMPILER_BYTE_ORDER;
>-  
>   _dbus_header_byteswap (&message->header, DBUS_COMPILER_BYTE_ORDER);
>+  _dbus_assert (_dbus_header_get_byte_order (&message->header) ==
>+                DBUS_COMPILER_BYTE_ORDER);
> }
> 
> /** byte-swap the message if it doesn't match our byte order.
>@@ -139,9 +142,7 @@ _dbus_message_byteswap (DBusMessage *message)
>  *  Otherwise should not be called since it would do needless
>  *  work.
>  */
>-#define ensure_byte_order(message)                      \
>- if (message->byte_order != DBUS_COMPILER_BYTE_ORDER)   \
>-   _dbus_message_byteswap (message)
>+#define ensure_byte_order(message) _dbus_message_byteswap (message)
> 
> /**
>  * Gets the data to be sent over the network for this message.
>@@ -661,15 +662,19 @@ dbus_message_cache_or_finalize (DBusMessage *message)
> static dbus_bool_t
> _dbus_message_iter_check (DBusMessageRealIter *iter)
> {
>+  char byte_order;
>+
>   if (iter == NULL)
>     {
>       _dbus_warn_check_failed ("dbus message iterator is NULL\n");
>       return FALSE;
>     }
> 
>+  byte_order = _dbus_header_get_byte_order (&iter->message->header);
>+
>   if (iter->iter_type == DBUS_MESSAGE_ITER_TYPE_READER)
>     {
>-      if (iter->u.reader.byte_order != iter->message->byte_order)
>+      if (iter->u.reader.byte_order != byte_order)
>         {
>           _dbus_warn_check_failed ("dbus message changed byte order since iterator was created\n");
>           return FALSE;
>@@ -679,7 +684,7 @@ _dbus_message_iter_check (DBusMessageRealIter *iter)
>     }
>   else if (iter->iter_type == DBUS_MESSAGE_ITER_TYPE_WRITER)
>     {
>-      if (iter->u.writer.byte_order != iter->message->byte_order)
>+      if (iter->u.writer.byte_order != byte_order)
>         {
>           _dbus_warn_check_failed ("dbus message changed byte order since append iterator was created\n");
>           return FALSE;
>@@ -1090,7 +1095,6 @@ dbus_message_new_empty_header (void)
>     }
>   
>   message->refcount.value = 1;
>-  message->byte_order = DBUS_COMPILER_BYTE_ORDER;
>   message->locked = FALSE;
> #ifndef DBUS_DISABLE_CHECKS
>   message->in_cache = FALSE;
>@@ -1110,12 +1114,12 @@ dbus_message_new_empty_header (void)
> 
>   if (from_cache)
>     {
>-      _dbus_header_reinit (&message->header, message->byte_order);
>+      _dbus_header_reinit (&message->header, DBUS_COMPILER_BYTE_ORDER);
>       _dbus_string_set_length (&message->body, 0);
>     }
>   else
>     {
>-      if (!_dbus_header_init (&message->header, message->byte_order))
>+      if (!_dbus_header_init (&message->header, DBUS_COMPILER_BYTE_ORDER))
>         {
>           dbus_free (message);
>           return NULL;
>@@ -1449,7 +1453,6 @@ dbus_message_copy (const DBusMessage *message)
>     return NULL;
> 
>   retval->refcount.value = 1;
>-  retval->byte_order = message->byte_order;
>   retval->locked = FALSE;
> #ifndef DBUS_DISABLE_CHECKS
>   retval->generation = message->generation;
>@@ -1929,7 +1932,7 @@ dbus_message_iter_init (DBusMessage     *message,
>                                   DBUS_MESSAGE_ITER_TYPE_READER);
> 
>   _dbus_type_reader_init (&real->u.reader,
>-                          message->byte_order,
>+                          _dbus_header_get_byte_order (&message->header),
>                           type_str, type_pos,
>                           &message->body,
>                           0);
>@@ -2284,7 +2287,7 @@ dbus_message_iter_init_append (DBusMessage     *message,
>    * due to OOM.
>    */
>   _dbus_type_writer_init_types_delayed (&real->u.writer,
>-                                        message->byte_order,
>+                                        _dbus_header_get_byte_order (&message->header),
>                                         &message->body,
>                                         _dbus_string_get_length (&message->body));
> }
>@@ -3995,8 +3998,6 @@ load_message (DBusMessageLoader *loader,
> 
>   _dbus_assert (validity == DBUS_VALID);
> 
>-  message->byte_order = byte_order;
>-
>   /* 2. VALIDATE BODY */
>   if (mode != DBUS_VALIDATION_MODE_WE_TRUST_THIS_DATA_ABSOLUTELY)
>     {
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 38287: 47948 | 47949

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.