Attachment 135635 Details for Bug 89104 – 02/12] DBusString: Add _DBUS_STRING_INIT_INVALID and allow "freeing" it

[patch] 02/12] DBusString: Add _DBUS_STRING_INIT_INVALID and allow "freeing" it

0002-DBusString-Add-_DBUS_STRING_INIT_INVALID-and-allow-f.patch (text/plain), 4.53 KB, created by Simon McVittie on 2017-11-21 16:20:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-11-21 16:20:55 UTC

Size: 4.53 KB

patch

obsolete

>From 1d50cda6b19c0368849bae082611058aec9dcb64 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Mon, 3 Jul 2017 17:59:10 +0100
>Subject: [PATCH 02/12] 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>
>---
> dbus/dbus-string-util.c | 10 +++++++++-
> dbus/dbus-string.c      | 23 ++++++++++++++++++++---
> dbus/dbus-string.h      | 16 ++++++++++++++++
> 3 files changed, 45 insertions(+), 4 deletions(-)
>
>diff --git a/dbus/dbus-string-util.c b/dbus/dbus-string-util.c
>index 08e0e917..a1390fea 100644
>--- 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 75134d6a..5d3ddda0 100644
>--- 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 e7060b9f..5e2d945e 100644
>--- 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.
>-- 
>2.15.0
>

Actions: View | Splinter Review

Attachments on bug 89104: 135264 | 135265 | 135634 | 135635 | 135636 | 135638 | 135639 | 135640 | 135641 | 135642 | 135643 | 135644 | 135645 | 135646 | 135738

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.