From 44c7eca26db612d6f90a98e832baa4125cc6fcc4 Mon Sep 17 00:00:00 2001 From: Simon McVittie 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 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