Skip to content

speculative generality in message (de)marshalling increases complexity

Submitted by Simon McVittie

Assigned to D-Bus Maintainers

Link to original bug (#38288)

Description

DBusTypeReader and DBusTypeWriter have some rather questionable functionality:

  • _dbus_type_reader_set_basic and _dbus_type_reader_delete allow a reader to be used for writing (!), used to edit header fields

  • DBusTypeWriter operates in terms of insertion, not appending, even though DBusMessage is append-only; as a result, it needs to be able to cope with realigning arbitrary types in arbitrary positions (even within an array!), which accounts for a large proportion of the complexity of marshal-recursive

  • when editing a message's header, the same very general code is used to realign all header fields after the insertion/deletion point, even though header fields are all already 8-byte-aligned due to being structs

  • even if we didn't realign, inserting or deleting in the middle of the header requires at least a memmove()

Here is a concrete proposal to simplify this:

  • Generalize dbus_write_*_two to take a (D-Bus abstraction representing a) struct iovec[] (I have a work-in-progress branch which adds DBusWriteVector, which is basically a struct iovec[] plus a count of bytes written so far)

  • If the platform is sufficiently rubbish that it doesn't have either sendmsg() or writev(), it can use a slow path involving repeated write() (Google suggests that on Windows we could use WSASend(), which is basically writev() NIH'd)

  • Stop viewing the message header as type (yyyyuua(uv)), and start viewing it as a 16-byte blob (yyyyuuu) (where the last u is really the length field for the a(uv), updated in the same sort of way as the body length), followed by some separate (uv) structs

  • Store each (uv) as a (string, start, len) tuple, where either:

    • the string is the original header string, edited fields are appended, and the fields they replaced are just not included in the struct iovec[]

    • the string is either the original header string, or (for edited fields) a second string used as scratch space

    • the string is either the original header string, or one extra string per edited field

  • Write messages into sockets by using scatter-gather I/O, with a struct iovec[n + 2] pointing to the 16-byte fixed part, the n header fields, and the body

  • Read messages into a single DBusString per header as we do now, and let all the header-field tuples point into that string

  • Delete all the code that requires realigning and be happy

Version: 1.5