DBusMessage: don't redundantly store byte order, ask the DBusHeader
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Fri, 10 Jun 2011 09:10:54 +0000 (10:10 +0100)
committerSimon McVittie <simon.mcvittie@collabora.co.uk>
Mon, 11 Jul 2011 16:54:36 +0000 (17:54 +0100)
Reviewed-by: Thiago Macieira <thiago@kde.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38287

dbus/dbus-marshal-header.c
dbus/dbus-marshal-header.h
dbus/dbus-message-factory.c
dbus/dbus-message-private.h
dbus/dbus-message.c

index a6c9b80..2fdf558 100644 (file)
@@ -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;
 }
 
 /** @} */
index fd16c5f..8713de3 100644 (file)
@@ -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);
 
 
 
index 7fae583..b5a06f1 100644 (file)
@@ -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)
index c5e3b3e..e64258c 100644 (file)
@@ -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
index a59ed9b..b208f52 100644 (file)
@@ -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)
     {