Re: [PATCH v2 05/11] dbus: GVariant message cookie and reply cookie are 64-bit

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

Hi Andrew,

On 04/11/2016 08:56 PM, Andrew Zaborowski wrote:
> They're 64-bit but the systemd docs discourage using the upper 32 bits,
> so we take advantage of that and only support cookie values that fit 32
> bits to avoid too big code changes (we use the dbus1 header layout for
> the cookie storage so we'd have to move the cookie/serial out of that
> header like we do with sender, path, etc.).  This only tries to fix the
> parsing and encoding.
> ---
>   ell/dbus-message.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 52 insertions(+), 8 deletions(-)
>
> diff --git a/ell/dbus-message.c b/ell/dbus-message.c
> index 964b925..eb47eb3 100644
> --- a/ell/dbus-message.c
> +++ b/ell/dbus-message.c
> @@ -111,13 +111,27 @@ void _dbus_message_set_serial(struct l_dbus_message *msg, uint32_t serial)
>   {
>   	struct dbus_header *hdr = msg->header;
>
> -	hdr->serial = serial;
> +	if (_dbus_message_is_gvariant(msg)) {
> +		if (_dbus_message_get_endian(msg) ==
> +				DBUS_MESSAGE_LITTLE_ENDIAN) {
> +			hdr->serial = serial;
> +			hdr->field_length = 0;
> +		} else {
> +			hdr->serial = 0;
> +			hdr->field_length = serial;
> +		}

This looks really complicated.  Can we just add a union to dbus_header 
instead to reflect the new GVariant serialization format?

> +	} else
> +		hdr->serial = serial;
>   }
>
>   uint32_t _dbus_message_get_serial(struct l_dbus_message *msg)
>   {
>   	struct dbus_header *hdr = msg->header;
>
> +	if (_dbus_message_is_gvariant(msg) && _dbus_message_get_endian(msg) ==
> +			DBUS_MESSAGE_BIG_ENDIAN)
> +		return hdr->field_length;
> +
>   	return hdr->serial;
>   }
>

Regards,
-Denis