DBusString: Add _DBUS_STRING_INIT_INVALID and allow "freeing" it

This means we can finally use patterns like this:

      DBusString buffer = _DBUS_STRING_INIT_INVALID;
      dbus_bool_t ret = FALSE;

      ... some long setup ...

      if (!_dbus_string_init (&buffer))
        goto out;

      ... some long operation ...

      ret = TRUE;

    out:
      ... free things ...
      _dbus_string_free (&buffer);
      ... free more things ...
      return ret;

without having to have a separate boolean to track whether buffer has
been initialized.

One observable difference is that if s is a "const" (borrowed pointer)
string, _dbus_string_free (&s) now sets it to be invalid. Previously,
it would have kept its (borrowed pointer) contents, which seems like
a violation of least-astonishment.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104

diff --git a/dbus/dbus-string-util.c b/dbus/dbus-string-util.c
index 08e0e917a267822602d51710e7d72837791a979d..a1390fea4f189657623bff2668875a69fd42e4fd 100644 (file)
--- a/dbus/dbus-string-util.c
+++ b/dbus/dbus-string-util.c
@@ -228,7 +228,7 @@ _DBUS_STRING_DEFINE_STATIC (test_static_string, "hello");
 dbus_bool_t
 _dbus_string_test (void)
 {
-  DBusString str;
+  DBusString str = _DBUS_STRING_INIT_INVALID;
   DBusString other;
   int i, a, end;
   long v;
@@ -245,6 +245,11 @@ _dbus_string_test (void)
   _dbus_assert (real_test_static_string->valid);
   _dbus_assert (real_test_static_string->align_offset == 0);
 
+  /* Test that _DBUS_STRING_INIT_INVALID has the desired effect */
+  _dbus_string_free (&str);
+  _dbus_string_free (&str);
+  _dbus_string_free (&str);
+
   /* Test shortening and setting length */
   i = 0;
   while (i < _DBUS_N_ELEMENTS (lens))
@@ -270,6 +275,9 @@ _dbus_string_test (void)
         }
       
       _dbus_string_free (&str);
+      /* Test that a cleared string is effectively _DBUS_STRING_INIT_INVALID */
+      _dbus_string_free (&str);
+      _dbus_string_free (&str);
 
       ++i;
     }

diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
index 75134d6a8b57efffc603a3bf2f1f0c93394fd5d0..5d3ddda00ad7aac204c52408cd6932f7a8934d1c 100644 (file)
--- a/dbus/dbus-string.c
+++ b/dbus/dbus-string.c
@@ -251,7 +251,12 @@ _dbus_string_init_from_string(DBusString       *str,
 }
 
 /**
- * Frees a string created by _dbus_string_init().
+ * Frees a string created by _dbus_string_init(), and fills it with the
+ * same contents as #_DBUS_STRING_INIT_INVALID.
+ *
+ * Unlike all other #DBusString API, it is also valid to call this function
+ * for a string that was filled with #_DBUS_STRING_INIT_INVALID.
+ * This is convenient for error rollback.
  *
  * @param str memory where the string is stored.
  */
@@ -259,20 +264,32 @@ void
 _dbus_string_free (DBusString *str)
 {
   DBusRealString *real = (DBusRealString*) str;
+  /* DBusRealString and DBusString have the same members in the same order,
+   * just differently-named */
+  DBusRealString invalid = _DBUS_STRING_INIT_INVALID;
+
+  /* Allow for the _DBUS_STRING_INIT_INVALID case */
+  if (real->str == NULL && real->len == 0 && real->allocated == 0 &&
+      !real->constant && !real->locked && !real->valid &&
+      real->align_offset == 0)
+    return;
+
   DBUS_GENERIC_STRING_PREAMBLE (real);
   
   if (real->constant)
-    return;
+    goto wipe;
 
   /* so it's safe if @p str returned by a failed
    * _dbus_string_init call
    * Bug: https://bugs.freedesktop.org/show_bug.cgi?id=65959
    */
   if (real->str == NULL)
-    return;
+    goto wipe;
 
   dbus_free (real->str - real->align_offset);
 
+wipe:
+  *real = invalid;
   real->valid = FALSE;
 }
 

diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
index e7060b9fb62db2585040fa55be3644227f9e39b6..5e2d945e35e020fb3e8b0421335137280c52806a 100644 (file)
--- a/dbus/dbus-string.h
+++ b/dbus/dbus-string.h
@@ -54,6 +54,22 @@ struct DBusString
   unsigned int dummy_bits : 3; /**< placeholder */
 };
 
+/**
+ * Content for a DBusString that is considered invalid for all
+ * operations, except that it is valid to call _dbus_string_free()
+ * during error handling.
+ */
+#define _DBUS_STRING_INIT_INVALID \
+{ \
+  NULL, /* dummy1 */ \
+  0, /* dummy2 */ \
+  0, /* dummy3 */ \
+  0, /* dummy_bit1 */ \
+  0, /* dummy_bit2 */ \
+  0, /* dummy_bit3 */ \
+  0 /* dummy_bits */ \
+}
+
 #ifdef DBUS_DISABLE_ASSERT
 /* Some simple inlining hacks; the current linker is not smart enough
  * to inline non-exported symbols across files in the library.