[PATCH] various fixes for marshlling empty fixed type arrays

[Rob Taylor]

Havoc Pennington wrote:
> Hi,
> 
> I think there's probably a better way to fix the dbus-marshal-basic.c
> issue (I'm not sure where it was breaking); the functions called there
> should allow a length of 0, so I'm guessing the problem was a NULL
> array_start or something? If so perhaps making dbus_string_copy allow a
> NULL string when length is 0 would be a nicer fix.
> 
> -  g_assert (msgarray != NULL);
> -  g_assert (msgarray_len >= 0);
> -  g_array_append_vals (ret, msgarray, (guint) msgarray_len);
> +  if (msgarray_len)
> 
> Suggestions:
>  - changing the first assert to msgarray != NULL || msgarray_len == 0 
>    would be nicer I think

Sounds good, new patch applied.

>  - why not leave the second assert there?
/me looks confused... leave it where?

>  - prefer msgarray_len != 0 for the test since it's not a bool
Agreed.

Modified patch attached.

> 
> -  _dbus_return_if_fail (dbus_type_is_fixed
> (_dbus_type_reader_get_current_type (&real->u.reader)));
> 
> Perhaps again the assertion could just be changed to
> len == 0 || (existing assertion)

Find attached a better solution. This uses the fact that
_dbus_type_reader_get_current_type will return DBUS_INVALID if the
reader is at the end of a block or end of a container.

> It's not redundant because assert and return_if_fail are different.
> return_if_fail is only used in public functions, and remains enabled in
> production code, while assertions are used to verify dbus's own internal
> correctness and are disabled in production code.
> 
> This return_if_fail would give you a semi-helpful warning if you tried
> to use the function on an array of non-fixed type, without it we
> probably just crash or something (in a production build without
> assertions)
> 

Hmm, that's a good point, I must have been half-sleepy when I posted
that patch.  Please find a saner solution attached.

Thanks,
Rob Taylor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-get-fixed-array-bogus-assertion.patch
Type: text/x-patch
Size: 894 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20060207/5b434e55/fix-get-fixed-array-bogus-assertion-0001.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: demarshal-collection-allow-empty-array.patch
Type: text/x-patch
Size: 673 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/dbus/attachments/20060207/5b434e55/demarshal-collection-allow-empty-array-0001.bin