lack of clarity in dbus-marshal-recursive.c: can type_str be NULL?
Submitted by Simon McVittie
Assigned to Simon McVittie
Description
+++ This bug was initially created as a clone of Bug #33128 +++
Attachment #42061 (a patch applied in Maemo) adds an assertion that writer->type_str can't be NULL in a particular part of the marshalling code, which was probably done to shut up a static analysis tool. It's not at all obvious whether the assertion is justified.
On closer inspection of the code, it seems that type_str can be NULL initially, but everything that uses it is guarded by extra initialization which ensures that it's non-NULL by copying in the message's signature. This is deferred until after dbus_message_iter_init, so that that function can't fail with OOM.
I think the right way to improve clarity here would be to rearrange the initialization so it's literally never NULL, then we can get rid of ~15 useless "is it NULL?" guards.
Possibilities include:
-
copy the signature in dbus_message_iter_init; if that fails, set an "already had OOM" flag which we check in all other accessors
-
defer initializing the writer until the point at which we currently copy in the signature
-
preallocate the DBusString for the signature as part of the DBusMessage
Version: 1.5