interfaces, object paths and activation
Mark McLoughlin
mark@skynet.ie
Mon, 22 Sep 2003 21:14:34 +0100
--=-7qz5Qg4f2ryvuFGidLXy
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Hi,
On Sun, 2003-09-21 at 19:36, Havoc Pennington wrote:
> On Sun, 2003-09-21 at 14:22, Mark McLoughlin wrote:
> > > One question I had - why align the field name to 4 byte boundaries?
> >
> > Yeah, good question. The old code did it this way too so I just made it
> > explicit. But sure, there isn't a good reason from a protocol point of
> > view to do it this way. I think, the only problem this will present is
> > making the field deletion logic cope with this ... but it might not be
> > such a bad idea to re-think that code anyway.
> >
> > Will I go ahead and do that, then ?
>
> The general rule has been to align everything naturally, so e.g. strings
> and bytes are aligned 1, 32-bit ints aligned 4, doubles aligned 8, etc.
> It makes sense to me to keep it that way, so a 1-byte field name would
> be aligned 1. If you want to code it up I'd be in favor, could save us
> some bytes and keep things consistent.
So here's a patch to do that. As I thought the header
deletion/modification code was the nasty bit (previously we were saved
by the fact that the field name and all field types were 4-byte aligned)
but it hasn't come out too bad.
Okay to commit ?
Cheers,
Mark.
--=-7qz5Qg4f2ryvuFGidLXy
Content-Disposition: attachment; filename=dbus-header-field-alignment.diff
Content-Type: text/x-patch; name=dbus-header-field-alignment.diff; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Index: ChangeLog
===================================================================
RCS file: /cvs/dbus/dbus/ChangeLog,v
retrieving revision 1.390.2.40
diff -u -p -r1.390.2.40 ChangeLog
--- ChangeLog 22 Sep 2003 05:45:59 -0000 1.390.2.40
+++ ChangeLog 22 Sep 2003 20:10:05 -0000
@@ -0,0 +1,62 @@
+2003-09-22 Mark McLoughlin <mark@skynet.ie>
+
+ * doc/dbus-specification.sgml: don't require header fields
+ to be 4-byte aligned and specify that fields should be
+ distinguished from padding by the fact that zero is not
+ a valid field name.
+
+ * doc/TODO: remove re-alignment item and add item to doc
+ the OBJECT_PATH type.
+
+ * dbus/dbus-message.c:
+ (HeaderField): rename the original member to value_offset
+ and introduce a name_offset member to keep track of where
+ the field actually begins.
+ (adjust_field_offsets): remove.
+ (append_int_field), (append_uint_field),
+ (append_string_field): don't align the start of the header
+ field to a 4-byte boundary.
+ (get_next_field): impl finding the next marhsalled field
+ after a given field.
+ (re_align_field_recurse): impl re-aligning a number of
+ already marshalled fields.
+ (delete_field): impl deleting a field of any type and
+ re-aligning any following fields.
+ (delete_int_or_uint_field), (delete_string_field): remove.
+ (set_int_field), (set_uint_field): no need to re-check
+ that we have the correct type for the field.
+ (set_string_field): ditto and impl re-aligning any
+ following fields.
+ (decode_header_data): update to take into account that
+ the fields aren't 4-byte aligned any more and the new
+ way to distinguish padding from header fields. Also,
+ don't exit when there is too much header padding.
+ (process_test_subdir): print the directory.
+ (_dbus_message_test): add test to make sure a following
+ field is re-aligned correctly after field deletion.
+
+ * dbus/dbus-string.[ch]:
+ (_dbus_string_insert_bytes): rename from insert_byte and
+ allow the insert of multiple bytes.
+ (_dbus_string_test): test inserting multiple bytes.
+
+ * dbus/dbus-marshal.c: (_dbus_marshal_set_string): add
+ warning note to docs about having to re-align any
+ marshalled values following the string.
+
+ * dbus/dbus-message-builder.c:
+ (append_string_field), (_dbus_message_data_load):
+ don't align the header field.
+
+ * dbus/dbus-auth.c: (process_test_subdir): print the
+ directory.
+
+ * test/break-loader.c: (randomly_add_one_byte): upd. for
+ insert_byte change.
+
+ * test/data/invalid-messages/bad-header-field-alignment.message:
+ new test case.
+
+ * test/data/valid-messages/unknown-header-field.message: shove
+ a dict in the unknown field.
+
Index: dbus/dbus-auth.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-auth.c,v
retrieving revision 1.28.2.2
diff -u -p -r1.28.2.2 dbus-auth.c
--- dbus/dbus-auth.c 7 Sep 2003 23:04:51 -0000 1.28.2.2
+++ dbus/dbus-auth.c 22 Sep 2003 20:10:05 -0000
@@ -2380,7 +2380,7 @@ process_test_subdir (const DBusString
goto failed;
}
- printf ("Testing:\n");
+ printf ("Testing %s:\n", subdir);
next:
while (_dbus_directory_get_next_file (dir, &filename, &error))
Index: dbus/dbus-marshal.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-marshal.c,v
retrieving revision 1.41.2.7
diff -u -p -r1.41.2.7 dbus-marshal.c
--- dbus/dbus-marshal.c 7 Sep 2003 23:04:52 -0000 1.41.2.7
+++ dbus/dbus-marshal.c 22 Sep 2003 20:10:05 -0000
@@ -395,6 +395,10 @@ _dbus_marshal_set_uint64 (DBusString
* an existing string or the wrong length will be deleted
* and replaced with the new string.
*
+ * Note: no attempt is made by this function to re-align
+ * any data which has been already marshalled after this
+ * string. Use with caution.
+ *
* @param str the string to write the marshalled string to
* @param offset the byte offset where string should be written
* @param byte_order the byte order to use
Index: dbus/dbus-message-builder.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-message-builder.c,v
retrieving revision 1.18.2.6
diff -u -p -r1.18.2.6 dbus-message-builder.c
--- dbus/dbus-message-builder.c 21 Sep 2003 18:43:20 -0000 1.18.2.6
+++ dbus/dbus-message-builder.c 22 Sep 2003 20:10:05 -0000
@@ -300,12 +300,6 @@ append_string_field (DBusString *dest,
{
int len;
- if (!_dbus_string_align_length (dest, 4))
- {
- _dbus_warn ("could not align field name\n");
- return FALSE;
- }
-
if (!_dbus_string_append_byte (dest, field))
{
_dbus_warn ("couldn't append field name byte\n");
@@ -710,11 +704,6 @@ _dbus_message_data_load (DBusString
goto parse_failed;
}
- if (unalign)
- unalign = FALSE;
- else
- _dbus_string_align_length (dest, 4);
-
if (!_dbus_string_append_byte (dest, field))
{
_dbus_warn ("could not append header field name byte\n");
Index: dbus/dbus-message.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-message.c,v
retrieving revision 1.102.2.11
diff -u -p -r1.102.2.11 dbus-message.c
--- dbus/dbus-message.c 21 Sep 2003 18:43:20 -0000 1.102.2.11
+++ dbus/dbus-message.c 22 Sep 2003 20:10:05 -0000
@@ -47,9 +47,8 @@
*/
typedef struct
{
- int offset; /**< Offset to start of field (location of name of field
- * for named fields)
- */
+ int name_offset; /**< Offset to name of field */
+ int value_offset; /**< Offset to value of field */
} HeaderField;
/** Offset to byte order from start of header */
@@ -183,26 +182,6 @@ append_header_padding (DBusMessage *mess
return TRUE;
}
-static void
-adjust_field_offsets (DBusMessage *message,
- int offsets_after,
- int delta)
-{
- int i;
-
- if (delta == 0)
- return;
-
- i = 0;
- while (i <= DBUS_HEADER_FIELD_LAST)
- {
- if (message->header_fields[i].offset > offsets_after)
- message->header_fields[i].offset += delta;
-
- ++i;
- }
-}
-
#ifdef DBUS_BUILD_TESTS
/* tests-only until it's actually used */
static dbus_int32_t
@@ -213,7 +192,7 @@ get_int_field (DBusMessage *message,
_dbus_assert (field <= DBUS_HEADER_FIELD_LAST);
- offset = message->header_fields[field].offset;
+ offset = message->header_fields[field].value_offset;
if (offset < 0)
return -1; /* useless if -1 is a valid value of course */
@@ -233,7 +212,7 @@ get_uint_field (DBusMessage *message,
_dbus_assert (field <= DBUS_HEADER_FIELD_LAST);
- offset = message->header_fields[field].offset;
+ offset = message->header_fields[field].value_offset;
if (offset < 0)
return -1; /* useless if -1 is a valid value of course */
@@ -252,7 +231,7 @@ get_string_field (DBusMessage *message,
int offset;
const char *data;
- offset = message->header_fields[field].offset;
+ offset = message->header_fields[field].value_offset;
_dbus_assert (field <= DBUS_HEADER_FIELD_LAST);
@@ -283,7 +262,7 @@ get_path_field_decomposed (DBusMessage
{
int offset;
- offset = message->header_fields[field].offset;
+ offset = message->header_fields[field].value_offset;
_dbus_assert (field <= DBUS_HEADER_FIELD_LAST);
@@ -306,16 +285,12 @@ append_int_field (DBusMessage *message,
int field,
int value)
{
- int orig_len;
-
_dbus_assert (!message->locked);
clear_header_padding (message);
- orig_len = _dbus_string_get_length (&message->header);
-
- if (!_dbus_string_align_length (&message->header, 4))
- goto failed;
+ message->header_fields[field].name_offset =
+ _dbus_string_get_length (&message->header);
if (!_dbus_string_append_byte (&message->header, field))
goto failed;
@@ -326,7 +301,7 @@ append_int_field (DBusMessage *message,
if (!_dbus_string_align_length (&message->header, 4))
goto failed;
- message->header_fields[field].offset =
+ message->header_fields[field].value_offset =
_dbus_string_get_length (&message->header);
if (!_dbus_marshal_int32 (&message->header, message->byte_order,
@@ -339,8 +314,10 @@ append_int_field (DBusMessage *message,
return TRUE;
failed:
- message->header_fields[field].offset = -1;
- _dbus_string_set_length (&message->header, orig_len);
+ _dbus_string_set_length (&message->header,
+ message->header_fields[field].name_offset);
+ message->header_fields[field].name_offset = -1;
+ message->header_fields[field].value_offset = -1;
/* this must succeed because it was allocated on function entry and
* DBusString doesn't ever realloc smaller
@@ -356,16 +333,12 @@ append_uint_field (DBusMessage *message,
int field,
int value)
{
- int orig_len;
-
_dbus_assert (!message->locked);
clear_header_padding (message);
- orig_len = _dbus_string_get_length (&message->header);
-
- if (!_dbus_string_align_length (&message->header, 4))
- goto failed;
+ message->header_fields[field].name_offset =
+ _dbus_string_get_length (&message->header);
if (!_dbus_string_append_byte (&message->header, field))
goto failed;
@@ -376,7 +349,7 @@ append_uint_field (DBusMessage *message,
if (!_dbus_string_align_length (&message->header, 4))
goto failed;
- message->header_fields[field].offset =
+ message->header_fields[field].value_offset =
_dbus_string_get_length (&message->header);
if (!_dbus_marshal_uint32 (&message->header, message->byte_order,
@@ -389,8 +362,10 @@ append_uint_field (DBusMessage *message,
return TRUE;
failed:
- message->header_fields[field].offset = -1;
- _dbus_string_set_length (&message->header, orig_len);
+ _dbus_string_set_length (&message->header,
+ message->header_fields[field].name_offset);
+ message->header_fields[field].name_offset = -1;
+ message->header_fields[field].value_offset = -1;
/* this must succeed because it was allocated on function entry and
* DBusString doesn't ever realloc smaller
@@ -406,16 +381,12 @@ append_string_field (DBusMessage *messag
int type,
const char *value)
{
- int orig_len;
-
_dbus_assert (!message->locked);
clear_header_padding (message);
- orig_len = _dbus_string_get_length (&message->header);
-
- if (!_dbus_string_align_length (&message->header, 4))
- goto failed;
+ message->header_fields[field].name_offset =
+ _dbus_string_get_length (&message->header);
if (!_dbus_string_append_byte (&message->header, field))
goto failed;
@@ -426,7 +397,7 @@ append_string_field (DBusMessage *messag
if (!_dbus_string_align_length (&message->header, 4))
goto failed;
- message->header_fields[field].offset =
+ message->header_fields[field].value_offset =
_dbus_string_get_length (&message->header);
if (!_dbus_marshal_string (&message->header, message->byte_order,
@@ -439,8 +410,10 @@ append_string_field (DBusMessage *messag
return TRUE;
failed:
- message->header_fields[field].offset = -1;
- _dbus_string_set_length (&message->header, orig_len);
+ _dbus_string_set_length (&message->header,
+ message->header_fields[field].name_offset);
+ message->header_fields[field].name_offset = -1;
+ message->header_fields[field].value_offset = -1;
/* this must succeed because it was allocated on function entry and
* DBusString doesn't ever realloc smaller
@@ -451,70 +424,137 @@ append_string_field (DBusMessage *messag
return FALSE;
}
-#ifdef DBUS_BUILD_TESTS
-/* This isn't used, but building it when tests are enabled just to
- * keep it compiling if we need it in future
- */
-static void
-delete_int_or_uint_field (DBusMessage *message,
- int field)
+static int
+get_next_field (DBusMessage *message,
+ int field)
{
- int offset = message->header_fields[field].offset;
+ int offset = message->header_fields[field].name_offset;
+ int closest;
+ int i;
+ int retval = DBUS_HEADER_FIELD_INVALID;
- _dbus_assert (!message->locked);
-
- if (offset < 0)
- return;
+ i = 0;
+ closest = _DBUS_INT_MAX;
+ while (i < DBUS_HEADER_FIELD_LAST)
+ {
+ if (message->header_fields[i].name_offset > offset &&
+ message->header_fields[i].name_offset < closest)
+ {
+ closest = message->header_fields[i].name_offset;
+ retval = i;
+ }
+ ++i;
+ }
- clear_header_padding (message);
-
- /* The field typecode and name take up 4 bytes */
- _dbus_string_delete (&message->header,
- offset - 4,
- 8);
-
- message->header_fields[field].offset = -1;
-
- adjust_field_offsets (message,
- offset - 4,
- - 8);
+ return retval;
+}
- append_header_padding (message);
+static void
+re_align_field_recurse (DBusMessage *message,
+ int field,
+ int offset)
+{
+ int old_name_offset = message->header_fields[field].name_offset;
+ int old_value_offset = message->header_fields[field].value_offset;
+ int prev_padding, padding, delta;
+ int type;
+ int next_field;
+ int pos = offset;
+
+ /* padding between the typecode byte and the value itself */
+ prev_padding = old_value_offset - old_name_offset + 2;
+
+ pos++;
+ type = _dbus_string_get_byte (&message->header, pos);
+
+ pos++;
+ switch (type)
+ {
+ case DBUS_TYPE_NIL:
+ case DBUS_TYPE_BYTE:
+ case DBUS_TYPE_BOOLEAN:
+ padding = 0;
+ break;
+ case DBUS_TYPE_INT32:
+ case DBUS_TYPE_UINT32:
+ case DBUS_TYPE_STRING:
+ case DBUS_TYPE_OBJECT_PATH:
+ padding = _DBUS_ALIGN_VALUE (pos, 4) - pos;
+ break;
+ case DBUS_TYPE_INT64:
+ case DBUS_TYPE_UINT64:
+ case DBUS_TYPE_DOUBLE:
+ padding = _DBUS_ALIGN_VALUE (pos, 8) - pos;
+ break;
+ case DBUS_TYPE_NAMED:
+ case DBUS_TYPE_ARRAY:
+ case DBUS_TYPE_DICT:
+ _dbus_assert_not_reached ("no defined header fields may contain a named, array or dict value");
+ break;
+ case DBUS_TYPE_INVALID:
+ default:
+ _dbus_assert_not_reached ("invalid type in marshalled header");
+ break;
+ }
+
+ delta = padding - prev_padding;
+ if (delta > 0)
+ _dbus_string_insert_bytes (&message->header, pos, delta, 0);
+ else if (delta < 0)
+ _dbus_string_delete (&message->header, pos, -delta);
+
+ pos += padding;
+
+ next_field = get_next_field (message, field);
+ if (next_field != DBUS_HEADER_FIELD_INVALID)
+ {
+ int next_offset = message->header_fields[next_field].name_offset;
+
+ _dbus_assert (next_offset > 0);
+
+ re_align_field_recurse (message, field,
+ pos + (next_offset - old_value_offset));
+ }
+
+ message->header_fields[field].name_offset = offset;
+ message->header_fields[field].value_offset = pos;
}
-#endif
static void
-delete_string_field (DBusMessage *message,
- int field)
+delete_field (DBusMessage *message,
+ int field)
{
- int offset = message->header_fields[field].offset;
- int len;
- int delete_len;
-
+ int offset = message->header_fields[field].name_offset;
+ int next_field;
+
_dbus_assert (!message->locked);
if (offset < 0)
- return;
+ return;
clear_header_padding (message);
-
- get_string_field (message, field, &len);
-
- /* The field typecode and name take up 4 bytes, and the nul
- * termination is 1 bytes, string length integer is 4 bytes
- */
- delete_len = 4 + 4 + 1 + len;
-
- _dbus_string_delete (&message->header,
- offset - 4,
- delete_len);
- message->header_fields[field].offset = -1;
-
- adjust_field_offsets (message,
- offset - 4,
- - delete_len);
+ next_field = get_next_field (message, field);
+ if (next_field == DBUS_HEADER_FIELD_INVALID)
+ {
+ _dbus_string_set_length (&message->header, offset);
+ }
+ else
+ {
+ int next_offset = message->header_fields[next_field].name_offset;
+ _dbus_assert (next_offset > 0);
+
+ _dbus_string_delete (&message->header,
+ offset,
+ next_offset - offset);
+
+ re_align_field_recurse (message, next_field, offset);
+ }
+
+ message->header_fields[field].name_offset = -1;
+ message->header_fields[field].value_offset = -1;
+
append_header_padding (message);
}
@@ -524,20 +564,14 @@ set_int_field (DBusMessage *message,
int field,
int value)
{
- int offset = message->header_fields[field].offset;
+ int offset = message->header_fields[field].value_offset;
_dbus_assert (!message->locked);
if (offset < 0)
{
/* need to append the field */
-
- switch (field)
- {
- default:
- _dbus_assert_not_reached ("appending an int field we don't support appending");
- return FALSE;
- }
+ return append_int_field (message, field, value);
}
else
{
@@ -555,23 +589,14 @@ set_uint_field (DBusMessage *message,
int field,
dbus_uint32_t value)
{
- int offset = message->header_fields[field].offset;
+ int offset = message->header_fields[field].value_offset;
_dbus_assert (!message->locked);
if (offset < 0)
{
/* need to append the field */
-
- switch (field)
- {
- case DBUS_HEADER_FIELD_REPLY_SERIAL:
- return append_uint_field (message, field, value);
-
- default:
- _dbus_assert_not_reached ("appending a uint field we don't support appending");
- return FALSE;
- }
+ return append_uint_field (message, field, value);
}
else
{
@@ -589,7 +614,7 @@ set_string_field (DBusMessage *message,
int type,
const char *value)
{
- int offset = message->header_fields[field].offset;
+ int offset = message->header_fields[field].value_offset;
_dbus_assert (!message->locked);
_dbus_assert (value != NULL);
@@ -597,32 +622,15 @@ set_string_field (DBusMessage *message,
if (offset < 0)
{
/* need to append the field */
-
- switch (field)
- {
- case DBUS_HEADER_FIELD_PATH:
- case DBUS_HEADER_FIELD_SENDER_SERVICE:
- case DBUS_HEADER_FIELD_INTERFACE:
- case DBUS_HEADER_FIELD_MEMBER:
- case DBUS_HEADER_FIELD_ERROR_NAME:
- case DBUS_HEADER_FIELD_SERVICE:
- return append_string_field (message, field, type, value);
-
- default:
- _dbus_assert_not_reached ("appending a string field we don't support appending");
- return FALSE;
- }
+ return append_string_field (message, field, type, value);
}
else
{
DBusString v;
- int old_len;
- int new_len;
int len;
+ int next_field;
clear_header_padding (message);
-
- old_len = _dbus_string_get_length (&message->header);
len = strlen (value);
@@ -633,12 +641,14 @@ set_string_field (DBusMessage *message,
offset, &v,
len))
goto failed;
-
- new_len = _dbus_string_get_length (&message->header);
- adjust_field_offsets (message,
- offset,
- new_len - old_len);
+ next_field = get_next_field (message, field);
+ if (next_field != DBUS_HEADER_FIELD_INVALID)
+ {
+ re_align_field_recurse (message,
+ next_field,
+ offset + 4 + len + 1);
+ }
if (!append_header_padding (message))
goto failed;
@@ -983,7 +993,8 @@ dbus_message_new_empty_header (void)
i = 0;
while (i <= DBUS_HEADER_FIELD_LAST)
{
- message->header_fields[i].offset = -1;
+ message->header_fields[i].name_offset = -1;
+ message->header_fields[i].value_offset = -1;
++i;
}
@@ -1281,7 +1292,7 @@ dbus_message_copy (const DBusMessage *me
for (i = 0; i <= DBUS_HEADER_FIELD_LAST; i++)
{
- retval->header_fields[i].offset = message->header_fields[i].offset;
+ retval->header_fields[i] = message->header_fields[i];
}
return retval;
@@ -1390,7 +1401,7 @@ dbus_message_set_path (DBusMessage *me
if (object_path == NULL)
{
- delete_string_field (message, DBUS_HEADER_FIELD_PATH);
+ delete_field (message, DBUS_HEADER_FIELD_PATH);
return TRUE;
}
else
@@ -1464,7 +1475,7 @@ dbus_message_set_interface (DBusMessage
if (interface == NULL)
{
- delete_string_field (message, DBUS_HEADER_FIELD_INTERFACE);
+ delete_field (message, DBUS_HEADER_FIELD_INTERFACE);
return TRUE;
}
else
@@ -1512,7 +1523,7 @@ dbus_message_set_member (DBusMessage *m
if (member == NULL)
{
- delete_string_field (message, DBUS_HEADER_FIELD_MEMBER);
+ delete_field (message, DBUS_HEADER_FIELD_MEMBER);
return TRUE;
}
else
@@ -1559,8 +1570,7 @@ dbus_message_set_error_name (DBusMessage
if (error_name == NULL)
{
- delete_string_field (message,
- DBUS_HEADER_FIELD_ERROR_NAME);
+ delete_field (message, DBUS_HEADER_FIELD_ERROR_NAME);
return TRUE;
}
else
@@ -1604,7 +1614,7 @@ dbus_message_set_destination (DBusMessag
if (destination == NULL)
{
- delete_string_field (message, DBUS_HEADER_FIELD_SERVICE);
+ delete_field (message, DBUS_HEADER_FIELD_SERVICE);
return TRUE;
}
else
@@ -4081,8 +4091,7 @@ dbus_message_set_sender (DBusMessage *m
if (sender == NULL)
{
- delete_string_field (message,
- DBUS_HEADER_FIELD_SENDER_SERVICE);
+ delete_field (message, DBUS_HEADER_FIELD_SENDER_SERVICE);
return TRUE;
}
else
@@ -4550,7 +4559,7 @@ decode_string_field (const DBusString
_dbus_assert (header_field != NULL);
_dbus_assert (field_data != NULL);
- if (header_field->offset >= 0)
+ if (header_field->name_offset >= 0)
{
_dbus_verbose ("%s field provided twice\n",
_dbus_header_field_to_string (field));
@@ -4575,12 +4584,13 @@ decode_string_field (const DBusString
_dbus_string_init_const (field_data,
_dbus_string_get_const_data (data) + string_data_pos);
- header_field->offset = _DBUS_ALIGN_VALUE (pos, 4);
+ header_field->name_offset = pos;
+ header_field->value_offset = _DBUS_ALIGN_VALUE (pos, 4);
#if 0
_dbus_verbose ("Found field %s at offset %d\n",
_dbus_header_field_to_string (field),
- header_field->offset);
+ header_field->value_offset);
#endif
return TRUE;
@@ -4609,29 +4619,18 @@ decode_header_data (const DBusString *
i = 0;
while (i <= DBUS_HEADER_FIELD_LAST)
{
- fields[i].offset = -1;
+ fields[i].name_offset = -1;
+ fields[i].value_offset = -1;
++i;
}
- /* Now handle the named fields. A real named field is at least 1
- * byte for the name, plus a type code (1 byte) plus padding, plus
- * the field value. So if we have less than 8 bytes left, it must
- * be alignment padding, not a field. While >= 8 bytes can't be
- * entirely alignment padding.
- */
pos = 16;
- while ((pos + 7) < header_len)
+ while (pos < header_len)
{
- pos = _DBUS_ALIGN_VALUE (pos, 4);
-
- if ((pos + 1) > header_len)
- {
- _dbus_verbose ("not enough space remains in header for header field value\n");
- return FALSE;
- }
-
field = _dbus_string_get_byte (data, pos);
- pos += 1;
+ if (field == DBUS_HEADER_FIELD_INVALID)
+ break; /* Must be padding */
+ pos++;
if (!_dbus_marshal_validate_type (data, pos, &type, &pos))
{
@@ -4737,7 +4736,7 @@ decode_header_data (const DBusString *
* type.
*/
- if (fields[field].offset >= 0)
+ if (fields[field].name_offset >= 0)
{
_dbus_verbose ("path field provided twice\n");
return FALSE;
@@ -4748,15 +4747,16 @@ decode_header_data (const DBusString *
return FALSE;
}
- fields[field].offset = _DBUS_ALIGN_VALUE (pos, 4);
+ fields[field].name_offset = pos;
+ fields[field].value_offset = _DBUS_ALIGN_VALUE (pos, 4);
/* No forging signals from the local path */
{
const char *s;
s = _dbus_string_get_const_data_len (data,
- fields[field].offset,
+ fields[field].value_offset,
_dbus_string_get_length (data) -
- fields[field].offset);
+ fields[field].value_offset);
if (strcmp (s, DBUS_PATH_ORG_FREEDESKTOP_LOCAL) == 0)
{
_dbus_verbose ("Message is on the local path\n");
@@ -4765,11 +4765,11 @@ decode_header_data (const DBusString *
}
_dbus_verbose ("Found path at offset %d\n",
- fields[field].offset);
+ fields[field].value_offset);
break;
case DBUS_HEADER_FIELD_REPLY_SERIAL:
- if (fields[field].offset >= 0)
+ if (fields[field].name_offset >= 0)
{
_dbus_verbose ("reply field provided twice\n");
return FALSE;
@@ -4781,10 +4781,11 @@ decode_header_data (const DBusString *
return FALSE;
}
- fields[field].offset = _DBUS_ALIGN_VALUE (pos, 4);
+ fields[field].name_offset = pos;
+ fields[field].value_offset = _DBUS_ALIGN_VALUE (pos, 4);
_dbus_verbose ("Found reply serial at offset %d\n",
- fields[field].offset);
+ fields[field].value_offset);
break;
default:
@@ -4798,7 +4799,11 @@ decode_header_data (const DBusString *
if (pos < header_len)
{
/* Alignment padding, verify that it's nul */
- _dbus_assert ((header_len - pos) < 8);
+ if ((header_len - pos) >= 8)
+ {
+ _dbus_verbose ("too much header alignment padding\n");
+ return FALSE;
+ }
if (!_dbus_string_validate_nul (data,
pos, (header_len - pos)))
@@ -4813,25 +4818,25 @@ decode_header_data (const DBusString *
{
case DBUS_MESSAGE_TYPE_SIGNAL:
case DBUS_MESSAGE_TYPE_METHOD_CALL:
- if (fields[DBUS_HEADER_FIELD_PATH].offset < 0)
+ if (fields[DBUS_HEADER_FIELD_PATH].value_offset < 0)
{
_dbus_verbose ("No path field provided\n");
return FALSE;
}
/* FIXME make this optional, at least for method calls */
- if (fields[DBUS_HEADER_FIELD_INTERFACE].offset < 0)
+ if (fields[DBUS_HEADER_FIELD_INTERFACE].value_offset < 0)
{
_dbus_verbose ("No interface field provided\n");
return FALSE;
}
- if (fields[DBUS_HEADER_FIELD_MEMBER].offset < 0)
+ if (fields[DBUS_HEADER_FIELD_MEMBER].value_offset < 0)
{
_dbus_verbose ("No member field provided\n");
return FALSE;
}
break;
case DBUS_MESSAGE_TYPE_ERROR:
- if (fields[DBUS_HEADER_FIELD_ERROR_NAME].offset < 0)
+ if (fields[DBUS_HEADER_FIELD_ERROR_NAME].value_offset < 0)
{
_dbus_verbose ("No error-name field provided\n");
return FALSE;
@@ -6069,7 +6074,7 @@ process_test_subdir (const DBusString
goto failed;
}
- printf ("Testing:\n");
+ printf ("Testing %s:\n", subdir);
next:
while (_dbus_directory_get_next_file (dir, &filename, &error))
@@ -6432,11 +6437,14 @@ _dbus_message_test (const char *test_dat
_dbus_assert (dbus_message_is_method_call (message, "Foo.TestInterface",
"TestMethod"));
_dbus_message_set_serial (message, 1234);
- dbus_message_set_sender (message, "org.foo.bar");
- _dbus_assert (dbus_message_has_sender (message, "org.foo.bar"));
+ /* string length including nul byte not a multiple of 4 */
+ dbus_message_set_sender (message, "org.foo.bar1");
+ _dbus_assert (dbus_message_has_sender (message, "org.foo.bar1"));
+ dbus_message_set_reply_serial (message, 5678);
dbus_message_set_sender (message, NULL);
- _dbus_assert (!dbus_message_has_sender (message, "org.foo.bar"));
+ _dbus_assert (!dbus_message_has_sender (message, "org.foo.bar1"));
_dbus_assert (dbus_message_get_serial (message) == 1234);
+ _dbus_assert (dbus_message_get_reply_serial (message) == 5678);
_dbus_assert (dbus_message_has_destination (message, "org.freedesktop.DBus.TestService"));
_dbus_assert (dbus_message_get_no_reply (message) == FALSE);
Index: dbus/dbus-string.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-string.c,v
retrieving revision 1.41.2.7
diff -u -p -r1.41.2.7 dbus-string.c
--- dbus/dbus-string.c 7 Sep 2003 23:04:52 -0000 1.41.2.7
+++ dbus/dbus-string.c 22 Sep 2003 20:10:05 -0000
@@ -562,26 +562,30 @@ _dbus_string_get_byte (const DBusString
}
/**
- * Inserts the given byte at the given position.
+ * Inserts a number of bytes of a given value at the
+ * given position.
*
* @param str the string
* @param i the position
+ * @param n_bytes number of bytes
* @param byte the value to insert
* @returns #TRUE on success
*/
dbus_bool_t
-_dbus_string_insert_byte (DBusString *str,
- int i,
- unsigned char byte)
+_dbus_string_insert_bytes (DBusString *str,
+ int i,
+ int n_bytes,
+ unsigned char byte)
{
DBUS_STRING_PREAMBLE (str);
_dbus_assert (i <= real->len);
_dbus_assert (i >= 0);
+ _dbus_assert (n_bytes > 0);
- if (!open_gap (1, real, i))
+ if (!open_gap (n_bytes, real, i))
return FALSE;
- real->str[i] = byte;
+ memset (real->str + i, byte, n_bytes);
return TRUE;
}
@@ -3572,23 +3576,26 @@ _dbus_string_test (void)
_dbus_string_set_byte (&str, 1, 'q');
_dbus_assert (_dbus_string_get_byte (&str, 1) == 'q');
- if (!_dbus_string_insert_byte (&str, 0, 255))
+ if (!_dbus_string_insert_bytes (&str, 0, 1, 255))
_dbus_assert_not_reached ("can't insert byte");
- if (!_dbus_string_insert_byte (&str, 2, 'Z'))
+ if (!_dbus_string_insert_bytes (&str, 2, 4, 'Z'))
_dbus_assert_not_reached ("can't insert byte");
- if (!_dbus_string_insert_byte (&str, _dbus_string_get_length (&str), 'W'))
+ if (!_dbus_string_insert_bytes (&str, _dbus_string_get_length (&str), 1, 'W'))
_dbus_assert_not_reached ("can't insert byte");
_dbus_assert (_dbus_string_get_byte (&str, 0) == 255);
_dbus_assert (_dbus_string_get_byte (&str, 1) == 'H');
_dbus_assert (_dbus_string_get_byte (&str, 2) == 'Z');
- _dbus_assert (_dbus_string_get_byte (&str, 3) == 'q');
- _dbus_assert (_dbus_string_get_byte (&str, 4) == 'l');
- _dbus_assert (_dbus_string_get_byte (&str, 5) == 'l');
- _dbus_assert (_dbus_string_get_byte (&str, 6) == 'o');
- _dbus_assert (_dbus_string_get_byte (&str, 7) == 'W');
+ _dbus_assert (_dbus_string_get_byte (&str, 3) == 'Z');
+ _dbus_assert (_dbus_string_get_byte (&str, 4) == 'Z');
+ _dbus_assert (_dbus_string_get_byte (&str, 5) == 'Z');
+ _dbus_assert (_dbus_string_get_byte (&str, 6) == 'q');
+ _dbus_assert (_dbus_string_get_byte (&str, 7) == 'l');
+ _dbus_assert (_dbus_string_get_byte (&str, 8) == 'l');
+ _dbus_assert (_dbus_string_get_byte (&str, 9) == 'o');
+ _dbus_assert (_dbus_string_get_byte (&str, 10) == 'W');
_dbus_string_free (&str);
Index: dbus/dbus-string.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-string.h,v
retrieving revision 1.20.2.4
diff -u -p -r1.20.2.4 dbus-string.h
--- dbus/dbus-string.h 7 Sep 2003 23:04:52 -0000 1.20.2.4
+++ dbus/dbus-string.h 22 Sep 2003 20:10:05 -0000
@@ -72,8 +72,9 @@ void _dbus_string_set_byte
unsigned char byte);
unsigned char _dbus_string_get_byte (const DBusString *str,
int start);
-dbus_bool_t _dbus_string_insert_byte (DBusString *str,
+dbus_bool_t _dbus_string_insert_bytes (DBusString *str,
int i,
+ int n_bytes,
unsigned char byte);
dbus_bool_t _dbus_string_steal_data (DBusString *str,
char **data_return);
Index: doc/TODO
===================================================================
RCS file: /cvs/dbus/dbus/doc/TODO,v
retrieving revision 1.22.2.8
diff -u -p -r1.22.2.8 TODO
--- doc/TODO 22 Sep 2003 03:11:12 -0000 1.22.2.8
+++ doc/TODO 22 Sep 2003 20:10:05 -0000
@@ -90,8 +90,6 @@
- document the auth protocol as a set of states and transitions, and
then reimplement it in those terms
- - Header fields names are required to be aligned on a 4 byte boundary
- at the moment. No alignment should be neccessary.
-
- dbus_gproxy or dbus_g_proxy?
+ - The OBJECT_PATH type is not documented in the spec.
Index: doc/dbus-specification.sgml
===================================================================
RCS file: /cvs/dbus/dbus/doc/dbus-specification.sgml,v
retrieving revision 1.39.2.7
diff -u -p -r1.39.2.7 dbus-specification.sgml
--- doc/dbus-specification.sgml 21 Sep 2003 18:43:20 -0000 1.39.2.7
+++ doc/dbus-specification.sgml 22 Sep 2003 20:10:05 -0000
@@ -265,11 +265,10 @@
</para>
<para>
- Header fields MUST be aligned to a 4-byte boundary. Header field
- names MUST consist of a single byte, possible values of which are
- defined below. Following the name, the field MUST have a type code
- represented as a single unsigned byte, and then a properly-aligned
- value of that type. See <xref
+ Header field names MUST consist of a single byte, possible values
+ of which are defined below. Following the name, the field MUST have
+ a type code represented as a single unsigned byte, and then a
+ properly-aligned value of that type. See <xref
linkend="message-protocol-arguments"> for a description of how each
type is encoded. If an implementation sees a header field name that
it does not understand, it MUST ignore that field.
@@ -358,10 +357,9 @@
buffer while keeping data types aligned, the total length of the header
must be a multiple of 8 bytes. To achieve this, the header MUST be padded
with nul bytes to align its total length on an 8-byte boundary.
- The minimum number of padding bytes MUST be used. Because all possible
- named fields use at least 8 bytes, implementations can distinguish
- padding (which must be less than 8 bytes) from additional named fields
- (which must be at least 8 bytes).
+ The minimum number of padding bytes MUST be used. Because zero is an
+ invalid field name, implementations can distinguish padding (which must be
+ zero initialized) from additional named fields.
</para>
</sect2>
Index: test/break-loader.c
===================================================================
RCS file: /cvs/dbus/dbus/test/break-loader.c,v
retrieving revision 1.7
diff -u -p -r1.7 break-loader.c
--- test/break-loader.c 22 Apr 2003 19:34:33 -0000 1.7
+++ test/break-loader.c 22 Sep 2003 20:10:05 -0000
@@ -287,8 +287,8 @@ randomly_add_one_byte (const DBusString
i = random_int_in_range (0, _dbus_string_get_length (mutated));
- _dbus_string_insert_byte (mutated, i,
- random_int_in_range (0, 256));
+ _dbus_string_insert_bytes (mutated, i, 1,
+ random_int_in_range (0, 256));
}
static void
Index: test/data/invalid-messages/bad-header-field-alignment.message
===================================================================
RCS file: test/data/invalid-messages/bad-header-field-alignment.message
diff -N test/data/invalid-messages/bad-header-field-alignment.message
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ test/data/invalid-messages/bad-header-field-alignment.message 22 Sep 2003 20:10:05 -0000
@@ -0,0 +1,34 @@
+## last field incorrectly aligned to 4 bytes
+
+## VALID_HEADER includes a LENGTH Header and LENGTH Body
+VALID_HEADER method_call
+
+HEADER_FIELD INTERFACE
+TYPE STRING
+STRING 'org.freedesktop.Foo'
+
+HEADER_FIELD MEMBER
+TYPE STRING
+STRING 'Bar'
+
+HEADER_FIELD UNKNOWN
+TYPE STRING
+STRING 'a'
+
+ALIGN 4
+
+HEADER_FIELD UNKNOWN
+TYPE ARRAY
+TYPE BYTE
+ALIGN 4
+LENGTH ThisByteArray
+START_LENGTH ThisByteArray
+BYTE 1
+BYTE 2
+END_LENGTH ThisByteArray
+
+
+ALIGN 8
+END_LENGTH Header
+START_LENGTH Body
+END_LENGTH Body
Index: test/data/valid-messages/unknown-header-field.message
===================================================================
RCS file: /cvs/dbus/dbus/test/data/valid-messages/unknown-header-field.message,v
retrieving revision 1.1.2.3
diff -u -p -r1.1.2.3 unknown-header-field.message
--- test/data/valid-messages/unknown-header-field.message 21 Sep 2003 18:43:20 -0000 1.1.2.3
+++ test/data/valid-messages/unknown-header-field.message 22 Sep 2003 20:10:05 -0000
@@ -3,9 +3,16 @@
## VALID_HEADER includes a LENGTH Header and LENGTH Body
VALID_HEADER method_call
REQUIRED_FIELDS
+
HEADER_FIELD UNKNOWN
+TYPE DICT
+LENGTH Dict
+START_LENGTH Dict
+STRING 'int32'
TYPE INT32
-INT32 0xfeeb
+INT32 0x12345678
+END_LENGTH Dict
+
ALIGN 8
END_LENGTH Header
START_LENGTH Body
--=-7qz5Qg4f2ryvuFGidLXy--