Remembering last offset and position at the message level

In GVariant, offsets in structs are stored for every end
of variable-sized elements except the last one.
We never know if an element we just added is the last one.
Therefore, the offset may be added when next element is added.
However, some users use multiple iterators. This change fixes
it at the root level - last offset value and position is kept
in the message.

Change-Id: I954657424d9da075edb10e5630b27df5bca8f917

diff --git a/dbus/dbus-marshal-gvariant.c b/dbus/dbus-marshal-gvariant.c
index 52595ed..932eff5 100644 (file)
--- a/dbus/dbus-marshal-gvariant.c
+++ b/dbus/dbus-marshal-gvariant.c
@@ -1154,38 +1154,37 @@ _dbus_writer_gvariant_add_offset_with_variability (DBusTypeWriter *writer,
 {
   writer->is_fixed = writer->is_fixed && fixed;
 
-  if (writer->body_container ||
-      DBUS_TYPE_STRUCT == writer->container_type ||
-      DBUS_TYPE_DICT_ENTRY == writer->container_type)
+  if (writer->body_container)
+  {
+    if (*writer->u.root.last_offset != 0)
+    {
+      check_offsets_in_body_for_adding (writer);
+
+      write_offset (writer->value_str,
+                    *writer->u.root.last_offset,
+                    writer->offsets_size,
+                    writer->value_pos);
+    }
+    if (!fixed)
+      *writer->u.root.last_offset = writer->value_pos - writer->value_start;
+    else
+      *writer->u.root.last_offset = 0;
+  }
+  else if (DBUS_TYPE_STRUCT == writer->container_type ||
+           DBUS_TYPE_DICT_ENTRY == writer->container_type)
   {
     if (writer->u.struct_or_dict.last_offset != 0)
     {
-      if (writer->body_container)
-      {
-        check_offsets_in_body_for_adding (writer);
+      check_offsets_for_adding (writer);
 
-        write_offset (writer->value_str,
+      prepend_offset (writer->offsets,
                       writer->u.struct_or_dict.last_offset,
-                      writer->offsets_size,
-                      writer->value_pos);
-      }
-      else
-      {
-        check_offsets_for_adding (writer);
-
-        prepend_offset (writer->offsets,
-                        writer->u.struct_or_dict.last_offset,
-                        writer->offsets_size);
-      }
+                      writer->offsets_size);
     }
     if (!fixed)
-    {
       writer->u.struct_or_dict.last_offset = writer->value_pos - writer->value_start;
-    }
     else
-    {
       writer->u.struct_or_dict.last_offset = 0;
-    }
   }
   else if (DBUS_TYPE_ARRAY == writer->container_type)
   {
@@ -1255,6 +1254,13 @@ fix_struct_alignment (DBusTypeWriter *writer, int type)
   return fix_struct_alignment_value (writer, get_alignment (type));
 }
 
+static void
+update_root_last_pos (DBusTypeWriter *writer)
+{
+  if (writer->body_container)
+    *writer->u.root.last_pos = writer->value_pos;
+}
+
 dbus_bool_t
 _dbus_type_writer_gvariant_write_basic_no_typecode (DBusTypeWriter *writer,
                                                     int             type,
@@ -1272,6 +1278,8 @@ _dbus_type_writer_gvariant_write_basic_no_typecode (DBusTypeWriter *writer,
                                                          writer->byte_order,
                                                          &writer->value_pos);
 
+  update_root_last_pos (writer);
+
   result = result && _dbus_writer_gvariant_add_offset (writer, type);
   return result;
 }
@@ -1364,6 +1372,8 @@ _dbus_writer_unrecurse_gvariant_write (DBusTypeWriter *writer,
       _dbus_assert_not_reached("Invalid container type");
   }
 
+  update_root_last_pos (writer);
+
   /* well, we don't know where in the type string beginning of current container is */
   result = result && _dbus_writer_gvariant_add_offset_with_variability (writer, sub->is_fixed);
 

diff --git a/dbus/dbus-marshal-recursive.c b/dbus/dbus-marshal-recursive.c
index 8a5c90e..ef41ffd 100644 (file)
--- a/dbus/dbus-marshal-recursive.c
+++ b/dbus/dbus-marshal-recursive.c
@@ -1697,12 +1697,26 @@ _dbus_type_writer_init_types_delayed (DBusTypeWriter *writer,
                           NULL, 0, value_str, value_pos);
 }
 
+/**
+ * Initialize a write iterator, with the signature to be provided
+ * later. Supports GVariant
+ *
+ * @param writer the writer to init
+ * @param byte_order the byte order to marshal into
+ * @param value_str the string to write values into
+ * @param value_pos where to insert values
+ * @param gvariant TRUE if append values with GVariant marshalling
+ * @param last_offset pointer to root level offset of last variable-size value
+ * @param last_pos pointer to root level position of offsets
+ */
 void
-_dbus_type_writer_gvariant_init_types_delayed (DBusTypeWriter *writer,
-                                      int             byte_order,
-                                      DBusString     *value_str,
-                                      int             value_pos,
-                                      dbus_bool_t     gvariant)
+_dbus_type_writer_gvariant_init_types_delayed (DBusTypeWriter  *writer,
+                                               int              byte_order,
+                                               DBusString      *value_str,
+                                               int              value_pos,
+                                               dbus_bool_t      gvariant,
+                                               size_t          *last_offset,
+                                               size_t          *last_pos)
 {
   _dbus_type_writer_init (writer, byte_order,
                           NULL, 0, value_str, value_pos);
@@ -1710,9 +1724,16 @@ _dbus_type_writer_gvariant_init_types_delayed (DBusTypeWriter *writer,
   writer->body_container = TRUE;
   writer->is_fixed = TRUE;
   writer->alignment = 8;
-  writer->u.struct_or_dict.last_offset = 0;
+  writer->u.root.last_offset = last_offset;
+  writer->u.root.last_pos = last_pos;
   writer->offsets_size = 1;
   writer->offsets = NULL;
+
+  if (gvariant)
+    {
+      writer->value_pos = *last_pos;
+      writer->value_start = 0;
+    }
 }
 
 /**
@@ -2003,7 +2024,8 @@ writer_recurse_struct_or_dict_entry (DBusTypeWriter   *writer,
                                           sub->value_pos,
                                           _DBUS_ALIGN_VALUE (sub->value_pos, 8) - sub->value_pos,
                                           '\0'))
-          _dbus_assert_not_reached ("should not have failed to insert alignment padding for struct");
+            _dbus_assert_not_reached ("should not have failed to insert alignment padding for struct");
+
           sub->value_pos =  _DBUS_ALIGN_VALUE (sub->value_pos, 8);
         }
     }

diff --git a/dbus/dbus-marshal-recursive.h b/dbus/dbus-marshal-recursive.h
index 0873c5b..1cb5205 100644 (file)
--- a/dbus/dbus-marshal-recursive.h
+++ b/dbus/dbus-marshal-recursive.h
@@ -102,6 +102,10 @@ struct DBusTypeWriter
     struct {
       size_t last_offset; /**< for GVariant marshalling: position of end of last field */
     } struct_or_dict;
+    struct {
+      size_t *last_offset; /**< for GVariant: pointer to root-level last offset */
+      size_t *last_pos;    /**< for GVariant: pointer to root-level last writing position */
+    } root;
   } u; /**< class-specific data */
 };
 
@@ -169,7 +173,9 @@ void        _dbus_type_writer_gvariant_init_types_delayed   (DBusTypeWriter
                                                              int                    byte_order,
                                                              DBusString            *value_str,
                                                              int                    value_pos,
-                                                             dbus_bool_t            gvariant);
+                                                             dbus_bool_t            gvariant,
+                                                             size_t                *last_offset,
+                                                             size_t                *last_pos);
 void        _dbus_type_writer_add_types            (DBusTypeWriter        *writer,
                                                     DBusString            *type_str,
                                                     int                    type_pos);

diff --git a/dbus/dbus-message-private.h b/dbus/dbus-message-private.h
index 674716a..d6f40da 100644 (file)
--- a/dbus/dbus-message-private.h
+++ b/dbus/dbus-message-private.h
@@ -132,6 +132,8 @@ struct DBusMessage
 #endif
   DBusString *signature; /**< A placeholder for signature of received GVariant messages */
   DBusString *unique_sender; /**< A placeholder for sender name of received GVariant messages */
+  size_t gvariant_body_last_offset; /**< Offset of end of last variable-sized element of body. */
+  size_t gvariant_body_last_pos; /**< Offset of last writing position. */
 };
 
 dbus_bool_t _dbus_message_iter_get_args_valist (DBusMessageIter *iter,

diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index e73741d..8d40716 100644 (file)
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -1382,6 +1382,8 @@ dbus_message_new_empty_header (dbus_bool_t gvariant)
 
   message->signature = NULL;
   message->unique_sender = NULL;
+  message->gvariant_body_last_offset = 0;
+  message->gvariant_body_last_pos = 0;
 
   return message;
 }
@@ -2549,7 +2551,9 @@ dbus_message_iter_init_append (DBusMessage     *message,
                               _dbus_header_get_byte_order (&message->header),
                               &message->body,
                               _dbus_string_get_length (&message->body),
-                              _dbus_message_is_gvariant (message));
+                              _dbus_message_is_gvariant (message),
+                              &message->gvariant_body_last_offset,
+                              &message->gvariant_body_last_pos);
 }
 
 /**

diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h
index 007a2d8..5597c48 100644 (file)
--- a/dbus/dbus-message.h
+++ b/dbus/dbus-message.h
@@ -113,6 +113,10 @@ struct DBusMessageIter
         struct {
           size_t dummy13;        /**< Don't use this */
         };
+        struct {
+          size_t *dummy14;       /**< Don't use this */
+          size_t *dummy15;       /**< Don't use this */
+        };
       } u;
     } s2;
   } u;