Commit 16b96009 authored by Simon McVittie's avatar Simon McVittie
Browse files

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: default avatarWill Thompson <will.thompson@collabora.co.uk>
parent e3b9a059
......@@ -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)
/**
......
......@@ -119,26 +119,6 @@ _dbus_string_find_byte_backward (const DBusString *str,
#include "dbus-test.h"
#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");
......
......@@ -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
......
......@@ -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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment