2005-01-17 Havoc Pennington <hp@redhat.com>

* Throughout, align variant bodies according to the contained
	type, rather than always to 8. Should save a fair bit of space in
	message headers.

	* dbus/dbus-marshal-validate.c (_dbus_validate_body_with_reason):
	fix handling of case where p == end

	* doc/TODO: remove the dbus_bool_t item and variant alignment items
This commit is contained in:
Havoc Pennington 2005-01-17 22:03:19 +00:00
parent 62e465339a
commit ad937e1695
7 changed files with 80 additions and 71 deletions

View file

@ -1,3 +1,14 @@
2005-01-17 Havoc Pennington <hp@redhat.com>
* Throughout, align variant bodies according to the contained
type, rather than always to 8. Should save a fair bit of space in
message headers.
* dbus/dbus-marshal-validate.c (_dbus_validate_body_with_reason):
fix handling of case where p == end
* doc/TODO: remove the dbus_bool_t item and variant alignment items
2005-01-17 Havoc Pennington <hp@redhat.com>
* dbus/dbus-types.h: hardcode dbus_bool_t to 32 bits

View file

@ -1389,6 +1389,30 @@ _dbus_verbose_bytes_of_string (const DBusString *str,
_dbus_verbose_bytes (d, len, start);
}
/**
* Get the first type in the signature. The difference between this
* and just getting the first byte of the signature is that you won't
* get DBUS_STRUCT_BEGIN_CHAR, you'll get DBUS_TYPE_STRUCT
* instead.
*
* @param str string containing signature
* @param pos where the signature starts
* @returns the first type in the signature
*/
int
_dbus_first_type_in_signature (const DBusString *str,
int pos)
{
unsigned char t;
t = _dbus_string_get_byte (str, pos);
if (t == DBUS_STRUCT_BEGIN_CHAR)
return DBUS_TYPE_STRUCT;
else
return t;
}
/** @} */
#ifdef DBUS_BUILD_TESTS

View file

@ -215,6 +215,7 @@ dbus_bool_t _dbus_type_is_container (int typecode);
dbus_bool_t _dbus_type_is_fixed (int typecode);
const char* _dbus_type_to_string (int typecode);
int _dbus_first_type_in_signature (const DBusString *str,
int pos);
#endif /* DBUS_MARSHAL_BASIC_H */

View file

@ -31,20 +31,6 @@
#include <stdio.h>
#include <stdlib.h>
static int
first_type_in_signature (const DBusString *str,
int pos)
{
unsigned char t;
t = _dbus_string_get_byte (str, pos);
if (t == DBUS_STRUCT_BEGIN_CHAR)
return DBUS_TYPE_STRUCT;
else
return t;
}
/* Whether to do the OOM stuff (only with other expensive tests) */
#define TEST_OOM_HANDLING 0
/* We do start offset 0 through 9, to get various alignment cases. Still this
@ -2678,7 +2664,7 @@ array_write_value (TestTypeNode *node,
&element_signature))
goto oom;
element_type = first_type_in_signature (&element_signature, 0);
element_type = _dbus_first_type_in_signature (&element_signature, 0);
if (!_dbus_type_writer_recurse (writer, DBUS_TYPE_ARRAY,
&element_signature, 0,

View file

@ -122,25 +122,11 @@ struct DBusTypeReaderClass
const DBusTypeMark *mark); /**< uncompress from a mark */
};
static int
first_type_in_signature (const DBusString *str,
int pos)
{
unsigned char t;
t = _dbus_string_get_byte (str, pos);
if (t == DBUS_STRUCT_BEGIN_CHAR)
return DBUS_TYPE_STRUCT;
else
return t;
}
static int
element_type_get_alignment (const DBusString *str,
int pos)
{
return _dbus_type_get_alignment (first_type_in_signature (str, pos));
return _dbus_type_get_alignment (_dbus_first_type_in_signature (str, pos));
}
static void
@ -265,7 +251,7 @@ array_reader_recurse (DBusTypeReader *sub,
sub->u.array.start_pos,
sub->array_len_offset,
array_reader_get_array_len (sub),
_dbus_type_to_string (first_type_in_signature (sub->type_str,
_dbus_type_to_string (_dbus_first_type_in_signature (sub->type_str,
sub->type_pos)));
#endif
}
@ -275,6 +261,7 @@ variant_reader_recurse (DBusTypeReader *sub,
DBusTypeReader *parent)
{
int sig_len;
int contained_alignment;
base_reader_recurse (sub, parent);
@ -289,7 +276,10 @@ variant_reader_recurse (DBusTypeReader *sub,
sub->value_pos = sub->type_pos + sig_len + 1;
sub->value_pos = _DBUS_ALIGN_VALUE (sub->value_pos, 8);
contained_alignment = _dbus_type_get_alignment (_dbus_first_type_in_signature (sub->type_str,
sub->type_pos));
sub->value_pos = _DBUS_ALIGN_VALUE (sub->value_pos, contained_alignment);
#if RECURSIVE_MARSHAL_READ_TRACE
_dbus_verbose (" type reader %p variant containing '%s'\n",
@ -429,7 +419,7 @@ base_reader_next (DBusTypeReader *reader,
{
if (!reader->klass->types_only)
_dbus_marshal_skip_array (reader->value_str,
first_type_in_signature (reader->type_str,
_dbus_first_type_in_signature (reader->type_str,
reader->type_pos + 1),
reader->byte_order,
&reader->value_pos);
@ -502,7 +492,7 @@ array_reader_next (DBusTypeReader *reader,
_dbus_assert (reader->value_pos < end_pos);
_dbus_assert (reader->value_pos >= reader->u.array.start_pos);
switch (first_type_in_signature (reader->type_str,
switch (_dbus_first_type_in_signature (reader->type_str,
reader->type_pos))
{
case DBUS_TYPE_STRUCT:
@ -527,7 +517,7 @@ array_reader_next (DBusTypeReader *reader,
case DBUS_TYPE_ARRAY:
{
_dbus_marshal_skip_array (reader->value_str,
first_type_in_signature (reader->type_str,
_dbus_first_type_in_signature (reader->type_str,
reader->type_pos + 1),
reader->byte_order,
&reader->value_pos);
@ -807,7 +797,7 @@ _dbus_type_reader_get_current_type (const DBusTypeReader *reader)
(* reader->klass->check_finished) (reader)))
t = DBUS_TYPE_INVALID;
else
t = first_type_in_signature (reader->type_str,
t = _dbus_first_type_in_signature (reader->type_str,
reader->type_pos);
_dbus_assert (t != DBUS_STRUCT_END_CHAR);
@ -837,7 +827,7 @@ _dbus_type_reader_get_element_type (const DBusTypeReader *reader)
_dbus_assert (_dbus_type_reader_get_current_type (reader) == DBUS_TYPE_ARRAY);
element_type = first_type_in_signature (reader->type_str,
element_type = _dbus_first_type_in_signature (reader->type_str,
reader->type_pos + 1);
return element_type;
@ -932,7 +922,7 @@ _dbus_type_reader_read_fixed_multi (const DBusTypeReader *reader,
_dbus_assert (!reader->klass->types_only);
_dbus_assert (reader->klass == &array_reader_class);
element_type = first_type_in_signature (reader->type_str,
element_type = _dbus_first_type_in_signature (reader->type_str,
reader->type_pos);
_dbus_assert (element_type != DBUS_TYPE_INVALID); /* why we don't use get_current_type() */
@ -989,7 +979,7 @@ _dbus_type_reader_recurse (DBusTypeReader *reader,
{
int t;
t = first_type_in_signature (reader->type_str, reader->type_pos);
t = _dbus_first_type_in_signature (reader->type_str, reader->type_pos);
switch (t)
{
@ -1649,7 +1639,7 @@ writer_recurse_init_and_check (DBusTypeWriter *writer,
{
int expected;
expected = first_type_in_signature (writer->type_str, writer->type_pos);
expected = _dbus_first_type_in_signature (writer->type_str, writer->type_pos);
if (expected != sub->container_type)
{
@ -1937,7 +1927,7 @@ writer_recurse_array (DBusTypeWriter *writer,
/* Variant value will normally have:
* 1 byte signature length not including nul
* signature typecodes (nul terminated)
* padding to 8-boundary
* padding to alignment of contained type
* body according to signature
*
* The signature string can only have a single type
@ -1945,21 +1935,12 @@ writer_recurse_array (DBusTypeWriter *writer,
*
* So a typical variant type with the integer 3 will have these
* octets:
* 0x1 'i' '\0' [padding to 8-boundary] 0x0 0x0 0x0 0x3
*
* For an array of 4-byte types stuffed into variants, the padding to
* 8-boundary is only the 1 byte that is required for the 4-boundary
* anyhow for all array elements after the first one. And for single
* variants in isolation, wasting a few bytes is hardly a big deal.
* 0x1 'i' '\0' [1 byte padding to alignment boundary] 0x0 0x0 0x0 0x3
*
* The main world of hurt for writing out a variant is that the type
* string is the same string as the value string. Which means
* inserting to the type string will move the value_pos; and it means
* that inserting to the type string could break type alignment.
*
* This type alignment issue is why the body of the variant is always
* 8-aligned. Then we know that re-8-aligning the start of the body
* will always correctly align the full contents of the variant type.
*/
static dbus_bool_t
writer_recurse_variant (DBusTypeWriter *writer,
@ -1968,6 +1949,8 @@ writer_recurse_variant (DBusTypeWriter *writer,
int contained_type_len,
DBusTypeWriter *sub)
{
int contained_alignment;
if (writer->enabled)
{
/* Allocate space for the worst case, which is 1 byte sig
@ -2018,12 +2001,14 @@ writer_recurse_variant (DBusTypeWriter *writer,
sub->value_pos += 1;
contained_alignment = _dbus_type_get_alignment (_dbus_first_type_in_signature (contained_type, contained_type_start));
if (!_dbus_string_insert_bytes (sub->value_str,
sub->value_pos,
_DBUS_ALIGN_VALUE (sub->value_pos, 8) - sub->value_pos,
_DBUS_ALIGN_VALUE (sub->value_pos, contained_alignment) - sub->value_pos,
'\0'))
_dbus_assert_not_reached ("should not have failed to insert alignment padding for variant body");
sub->value_pos = _DBUS_ALIGN_VALUE (sub->value_pos, 8);
sub->value_pos = _DBUS_ALIGN_VALUE (sub->value_pos, contained_alignment);
return TRUE;
}

View file

@ -304,7 +304,10 @@ validate_body_helper (DBusTypeReader *reader,
case DBUS_TYPE_VARIANT:
{
/* 1 byte sig len, sig typecodes, align to 8-boundary, values. */
/* 1 byte sig len, sig typecodes, align to
* contained-type-boundary, values.
*/
/* In addition to normal signature validation, we need to be sure
* the signature contains only a single (possibly container) type.
*/
@ -312,6 +315,7 @@ validate_body_helper (DBusTypeReader *reader,
DBusString sig;
DBusTypeReader sub;
DBusValidity validity;
int contained_alignment;
claimed_len = *p;
++p;
@ -331,7 +335,9 @@ validate_body_helper (DBusTypeReader *reader,
return DBUS_INVALID_VARIANT_SIGNATURE_MISSING_NUL;
++p;
a = _DBUS_ALIGN_ADDRESS (p, 8);
contained_alignment = _dbus_type_get_alignment (_dbus_first_type_in_signature (&sig, 0));
a = _DBUS_ALIGN_ADDRESS (p, contained_alignment);
if (a > end)
return DBUS_INVALID_NOT_ENOUGH_DATA;
while (p != a)
@ -460,16 +466,19 @@ _dbus_validate_body_with_reason (const DBusString *expected_signature,
validity = validate_body_helper (&reader, byte_order, TRUE, p, end, &p);
if (validity != DBUS_VALID)
return validity;
if (p < end)
if (bytes_remaining)
{
if (bytes_remaining)
*bytes_remaining = end - p;
else
return DBUS_INVALID_TOO_MUCH_DATA;
*bytes_remaining = end - p;
return DBUS_VALID;
}
else if (p < end)
return DBUS_INVALID_TOO_MUCH_DATA;
else
{
_dbus_assert (p == end);
return DBUS_VALID;
}
return DBUS_VALID;
}
/**

View file

@ -38,13 +38,6 @@ Important for 1.0
(though they are kind of a pita to pass in as size_t with the
varargs, so maybe not - what does glib do with g_object_get()?)
- it's probably obnoxious that reading/writing bools doesn't return
dbus_bool_t; solution is probably to change bool to 32 bits on the
wire
- maybe change and don't align variant bodies to 8-boundary, it uses
up lots of space in a typical header
- rename the service thing. unique service names (":1") and well-known
("org.foo.bar") should have different names probably; something like
"address" for the unique and "alias" for the well-known, or