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.)

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=38570
Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>

4 files changed, 16 insertions, 105 deletions

diff --git a/dbus/dbus-string-private.h b/dbus/dbus-string-private.h
index 500e0b773..185515d89 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 b31703ca6..421ac1b48 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 e2a7e0832..b1fa59862 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 2f1ed31c2..2f1fe8787 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