lack of clarity in dbus-marshal-recursive.c: can type_str be NULL?
Submitted by Simon McVittie
Assigned to Simon McVittie
+++ 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.
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