Attachment 48287 Details for Bug 38570 – [PATCH 3/3] Remove maximum length field from DBusString

[patch] [PATCH 3/3] Remove maximum length field from DBusString

0003-Remove-maximum-length-field-from-DBusString.patch (text/plain), 11.55 KB, created by Simon McVittie on 2011-06-22 07:01:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-06-22 07:01:45 UTC

Size: 11.55 KB

patch

obsolete

>From 44c7eca26db612d6f90a98e832baa4125cc6fcc4 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Wed, 22 Jun 2011 14:57:56 +0100
>Subject: [PATCH 3/3] Remove maximum length field from DBusString
>
>The source code says it's "a historical artifact from a feature that
>turned out to be dumb". Respond accordingly!
>
>This reduces sizeof (DBusString) by 20% on ILP32 architectures, which
>can't hurt. (No reduction on LP64 architectures that align pointers
>naturally, unfortunately.)
>---
> dbus/dbus-string-private.h |    9 ++----
> dbus/dbus-string-util.c    |   55 --------------------------------------------
> dbus/dbus-string.c         |   47 +++++++------------------------------
> dbus/dbus-string.h         |   10 +++----
> 4 files changed, 16 insertions(+), 105 deletions(-)
>
>diff --git a/dbus/dbus-string-private.h b/dbus/dbus-string-private.h
>index 500e0b7..185515d 100644
>--- a/dbus/dbus-string-private.h
>+++ b/dbus/dbus-string-private.h
>@@ -44,7 +44,6 @@ typedef struct
>   unsigned char *str;            /**< String data, plus nul termination */
>   int            len;            /**< Length without nul */
>   int            allocated;      /**< Allocated size of data */
>-  int            max_length;     /**< Max length of this string, without nul byte */
>   unsigned int   constant : 1;   /**< String data is not owned by DBusString */
>   unsigned int   locked : 1;     /**< DBusString has been locked and can't be changed */
>   unsigned int   invalid : 1;    /**< DBusString is invalid (e.g. already freed) */
>@@ -63,10 +62,9 @@ typedef struct
>  */
> 
> /**
>- * This is the maximum max length (and thus also the maximum length)
>- * of a DBusString
>+ * The maximum length of a DBusString
>  */
>-#define _DBUS_STRING_MAX_MAX_LENGTH (_DBUS_INT32_MAX - _DBUS_STRING_ALLOCATION_PADDING)
>+#define _DBUS_STRING_MAX_LENGTH (_DBUS_INT32_MAX - _DBUS_STRING_ALLOCATION_PADDING)
> 
> /**
>  * Checks a bunch of assertions about a string object
>@@ -79,9 +77,8 @@ typedef struct
>       _dbus_assert (!(real)->invalid); \
>       _dbus_assert ((real)->len >= 0); \
>       _dbus_assert ((real)->allocated >= 0); \
>-      _dbus_assert ((real)->max_length >= 0); \
>       _dbus_assert ((real)->len <= ((real)->allocated - _DBUS_STRING_ALLOCATION_PADDING)); \
>-      _dbus_assert ((real)->len <= (real)->max_length); \
>+      _dbus_assert ((real)->len <= _DBUS_STRING_MAX_LENGTH); \
>   } while (0)
> 
> /**
>diff --git a/dbus/dbus-string-util.c b/dbus/dbus-string-util.c
>index b31703c..421ac1b 100644
>--- a/dbus/dbus-string-util.c
>+++ b/dbus/dbus-string-util.c
>@@ -120,26 +120,6 @@ _dbus_string_find_byte_backward (const DBusString  *str,
> #include <stdio.h>
> 
> static void
>-test_max_len (DBusString *str,
>-              int         max_len)
>-{
>-  if (max_len > 0)
>-    {
>-      if (!_dbus_string_set_length (str, max_len - 1))
>-        _dbus_assert_not_reached ("setting len to one less than max should have worked");
>-    }
>-
>-  if (!_dbus_string_set_length (str, max_len))
>-    _dbus_assert_not_reached ("setting len to max len should have worked");
>-
>-  if (_dbus_string_set_length (str, max_len + 1))
>-    _dbus_assert_not_reached ("setting len to one more than max len should not have worked");
>-
>-  if (!_dbus_string_set_length (str, 0))
>-    _dbus_assert_not_reached ("setting len to zero should have worked");
>-}
>-
>-static void
> test_hex_roundtrip (const unsigned char *data,
>                     int                  len)
> {
>@@ -232,25 +212,6 @@ test_roundtrips (TestRoundtripFunc func)
>   }
> }
> 
>-#ifdef DBUS_BUILD_TESTS
>-/* The max length thing is sort of a historical artifact
>- * from a feature that turned out to be dumb; perhaps
>- * we should purge it entirely. The problem with
>- * the feature is that it looks like memory allocation
>- * failure, but is not a transient or resolvable failure.
>- */
>-static void
>-set_max_length (DBusString *str,
>-                int         max_length)
>-{
>-  DBusRealString *real;
>-  
>-  real = (DBusRealString*) str;
>-
>-  real->max_length = max_length;
>-}
>-#endif /* DBUS_BUILD_TESTS */
>-
> /**
>  * @ingroup DBusStringInternals
>  * Unit test for DBusString.
>@@ -272,20 +233,6 @@ _dbus_string_test (void)
>   int lens[] = { 0, 1, 2, 3, 4, 5, 10, 16, 17, 18, 25, 31, 32, 33, 34, 35, 63, 64, 65, 66, 67, 68, 69, 70, 71, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136 };
>   char *s;
>   dbus_unichar_t ch;
>-  
>-  i = 0;
>-  while (i < _DBUS_N_ELEMENTS (lens))
>-    {
>-      if (!_dbus_string_init (&str))
>-        _dbus_assert_not_reached ("failed to init string");
>-
>-      set_max_length (&str, lens[i]);
>-      
>-      test_max_len (&str, lens[i]);
>-      _dbus_string_free (&str);
>-
>-      ++i;
>-    }
> 
>   /* Test shortening and setting length */
>   i = 0;
>@@ -296,8 +243,6 @@ _dbus_string_test (void)
>       if (!_dbus_string_init (&str))
>         _dbus_assert_not_reached ("failed to init string");
> 
>-      set_max_length (&str, lens[i]);
>-      
>       if (!_dbus_string_set_length (&str, lens[i]))
>         _dbus_assert_not_reached ("failed to set string length");
> 
>diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c
>index e2a7e08..b1fa598 100644
>--- a/dbus/dbus-string.c
>+++ b/dbus/dbus-string.c
>@@ -154,7 +154,6 @@ _dbus_string_init_preallocated (DBusString *str,
>   real->len = 0;
>   real->str[real->len] = '\0';
>   
>-  real->max_length = _DBUS_STRING_MAX_MAX_LENGTH;
>   real->constant = FALSE;
>   real->locked = FALSE;
>   real->invalid = FALSE;
>@@ -178,25 +177,6 @@ _dbus_string_init (DBusString *str)
>   return _dbus_string_init_preallocated (str, 0);
> }
> 
>-#ifdef DBUS_BUILD_TESTS
>-/* The max length thing is sort of a historical artifact
>- * from a feature that turned out to be dumb; perhaps
>- * we should purge it entirely. The problem with
>- * the feature is that it looks like memory allocation
>- * failure, but is not a transient or resolvable failure.
>- */
>-static void
>-set_max_length (DBusString *str,
>-                int         max_length)
>-{
>-  DBusRealString *real;
>-  
>-  real = (DBusRealString*) str;
>-
>-  real->max_length = max_length;
>-}
>-#endif /* DBUS_BUILD_TESTS */
>-
> /**
>  * Initializes a constant string. The value parameter is not copied
>  * (should be static), and the string may never be modified.
>@@ -235,7 +215,7 @@ _dbus_string_init_const_len (DBusString *str,
>   
>   _dbus_assert (str != NULL);
>   _dbus_assert (len == 0 || value != NULL);
>-  _dbus_assert (len <= _DBUS_STRING_MAX_MAX_LENGTH);
>+  _dbus_assert (len <= _DBUS_STRING_MAX_LENGTH);
>   _dbus_assert (len >= 0);
>   
>   real = (DBusRealString*) str;
>@@ -243,7 +223,6 @@ _dbus_string_init_const_len (DBusString *str,
>   real->str = (unsigned char*) value;
>   real->len = len;
>   real->allocated = real->len + _DBUS_STRING_ALLOCATION_PADDING; /* a lie, just to avoid special-case assertions... */
>-  real->max_length = real->len + 1;
>   real->constant = TRUE;
>   real->locked = TRUE;
>   real->invalid = FALSE;
>@@ -336,8 +315,8 @@ reallocate_for_length (DBusRealString *real,
>   /* at least double our old allocation to avoid O(n), avoiding
>    * overflow
>    */
>-  if (real->allocated > (_DBUS_STRING_MAX_MAX_LENGTH + _DBUS_STRING_ALLOCATION_PADDING) / 2)
>-    new_allocated = _DBUS_STRING_MAX_MAX_LENGTH + _DBUS_STRING_ALLOCATION_PADDING;
>+  if (real->allocated > (_DBUS_STRING_MAX_LENGTH + _DBUS_STRING_ALLOCATION_PADDING) / 2)
>+    new_allocated = _DBUS_STRING_MAX_LENGTH + _DBUS_STRING_ALLOCATION_PADDING;
>   else
>     new_allocated = real->allocated * 2;
> 
>@@ -400,7 +379,7 @@ set_length (DBusRealString *real,
>   /* Note, we are setting the length not including nul termination */
> 
>   /* exceeding max length is the same as failure to allocate memory */
>-  if (_DBUS_UNLIKELY (new_length > real->max_length))
>+  if (_DBUS_UNLIKELY (new_length > _DBUS_STRING_MAX_LENGTH))
>     return FALSE;
>   else if (new_length > (real->allocated - _DBUS_STRING_ALLOCATION_PADDING) &&
>            _DBUS_UNLIKELY (!reallocate_for_length (real, new_length)))
>@@ -421,7 +400,7 @@ open_gap (int             len,
>   if (len == 0)
>     return TRUE;
> 
>-  if (len > dest->max_length - dest->len)
>+  if (len > _DBUS_STRING_MAX_LENGTH - dest->len)
>     return FALSE; /* detected overflow of dest->len + len below */
>   
>   if (!set_length (dest, dest->len + len))
>@@ -640,7 +619,6 @@ dbus_bool_t
> _dbus_string_steal_data (DBusString        *str,
>                          char             **data_return)
> {
>-  int old_max_length;
>   DBUS_STRING_PREAMBLE (str);
>   _dbus_assert (data_return != NULL);
> 
>@@ -648,8 +626,6 @@ _dbus_string_steal_data (DBusString        *str,
>   
>   *data_return = (char*) real->str;
> 
>-  old_max_length = real->max_length;
>-  
>   /* reset the string */
>   if (!_dbus_string_init (str))
>     {
>@@ -660,8 +636,6 @@ _dbus_string_steal_data (DBusString        *str,
>       return FALSE;
>     }
> 
>-  real->max_length = old_max_length;
>-
>   return TRUE;
> }
> 
>@@ -767,7 +741,7 @@ _dbus_string_lengthen (DBusString *str,
>   DBUS_STRING_PREAMBLE (str);  
>   _dbus_assert (additional_length >= 0);
> 
>-  if (_DBUS_UNLIKELY (additional_length > real->max_length - real->len))
>+  if (_DBUS_UNLIKELY (additional_length > _DBUS_STRING_MAX_LENGTH - real->len))
>     return FALSE; /* would overflow */
>   
>   return set_length (real,
>@@ -833,7 +807,7 @@ align_insert_point_then_open_gap (DBusString *str,
>   gap_pos = _DBUS_ALIGN_VALUE (insert_at, alignment);
>   new_len = real->len + (gap_pos - insert_at) + gap_size;
>   
>-  if (_DBUS_UNLIKELY (new_len > (unsigned long) real->max_length))
>+  if (_DBUS_UNLIKELY (new_len > (unsigned long) _DBUS_STRING_MAX_LENGTH))
>     return FALSE;
>   
>   delta = new_len - real->len;
>@@ -945,7 +919,7 @@ _dbus_string_append (DBusString *str,
>   _dbus_assert (buffer != NULL);
>   
>   buffer_len = strlen (buffer);
>-  if (buffer_len > (unsigned long) real->max_length)
>+  if (buffer_len > (unsigned long) _DBUS_STRING_MAX_LENGTH)
>     return FALSE;
>   
>   return append (real, buffer, buffer_len);
>@@ -1289,7 +1263,7 @@ _dbus_string_append_unichar (DBusString    *str,
>       len = 6;
>     }
> 
>-  if (len > (real->max_length - real->len))
>+  if (len > (_DBUS_STRING_MAX_LENGTH - real->len))
>     return FALSE; /* real->len + len would overflow */
>   
>   if (!set_length (real, real->len + len))
>@@ -1438,9 +1412,6 @@ _dbus_string_copy (const DBusString *source,
>  * Like _dbus_string_move(), but can move a segment from
>  * the middle of the source string.
>  *
>- * @todo this doesn't do anything with max_length field.
>- * we should probably just kill the max_length field though.
>- * 
>  * @param source the source string
>  * @param start first byte of source string to move
>  * @param len length of segment to move
>diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h
>index 2f1ed31..2f1fe87 100644
>--- a/dbus/dbus-string.h
>+++ b/dbus/dbus-string.h
>@@ -48,11 +48,10 @@ struct DBusString
> #endif
>   int   dummy2;       /**< placeholder */
>   int   dummy3;       /**< placeholder */
>-  int   dummy4;       /**< placeholder */
>-  unsigned int dummy5 : 1; /**< placeholder */
>-  unsigned int dummy6 : 1; /**< placeholder */
>-  unsigned int dummy7 : 1; /**< placeholder */
>-  unsigned int dummy8 : 3; /**< placeholder */
>+  unsigned int dummy_bit1 : 1; /**< placeholder */
>+  unsigned int dummy_bit2 : 1; /**< placeholder */
>+  unsigned int dummy_bit3 : 1; /**< placeholder */
>+  unsigned int dummy_bits : 3; /**< placeholder */
> };
> 
> #ifdef DBUS_DISABLE_ASSERT
>@@ -324,7 +323,6 @@ void          _dbus_string_zero                  (DBusString        *str);
>                                    sizeof(_dbus_static_string_##name),  \
>                                    sizeof(_dbus_static_string_##name) + \
>                                    _DBUS_STRING_ALLOCATION_PADDING,     \
>-                                   sizeof(_dbus_static_string_##name),  \
>                                    TRUE, TRUE, FALSE, 0 }
> 
> DBUS_END_DECLS
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 38570: 48285 | 48286 | 48287

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.