Bug 11191

I'm re-purposing this bug as "make DBusBasicValue visible as API" because:

(In reply to comment #0)
> If you really think that a) copying basic types generically is
> more than just a random example that nobody is actually going to use
> b) such platforms matter (I don't think they exist. People use GCC
> on obscure platforms), just do:
> 
>  char value[8];
> 
> To make it explicit that you want 8 uninterpreted 
> bytes, and again skip the chatter :-)

Strictly speaking, for libdbus' current usage it needs to be a union { char[8]; char * } because char * can theoretically be larger than 64 bits (yeah yeah "that'll never happen", but 640k ought to be enough for anyone too).

However:

* We already have a perfectly good union of all allowed types, called
  DBusBasicValue. It's not public API but we could make it so.

* Every libdbus binding that isn't remotely exploitable has to have a
  great big switch() over all the basic types and remap them into the
  desired language types, and these often benefit from having a union
  that looks suspiciously like DBusBasicValue (e.g. see
  _dbus_bindings/message-*.c in dbus-python for two such unions).
  dbus_message_iter_get_basic() effectively takes a DBusBasicValue *
  already, it just can't admit it because that type isn't API
  (and the caller can supply a smaller or less-aligned buffer if they
  know the exact type they're extracting).

So, let's just make DBusBasicValue public and suggest using it as the argument of dbus_message_iter_get_basic() and friends.

I'm very tempted to do this in 1.4 even though it's new API, because it doesn't add ABI, and is really just formalizing the API/ABI that we've had since 0.something.

Created attachment 53577 [details] [review]
[1/2] Promote DBusBasicValue and DBus8ByteStruct to be API

In practice, D-Bus bindings end up reinventing DBusBasicValue anyway,
so it might as well be API.

Also stop claiming that all basic-typed values are guaranteed to fit in
8 bytes - this is not true if your platform has more than 8-byte pointers
(I'm not aware of any such platform now, but let's not rule it out).

Created attachment 53578 [details] [review]
[2/2] DBusBasicValue: add bool_val and fd members to complete the set

dbus_bool_t is the same as dbus_uint32_t, but if we have a separate
bool_val member, it's more obvious that people are getting it right.
It's not called bool because that's a keyword in C++.

int (for file descriptors) doesn't appear in the D-Bus message wire
format, but then again neither does char *, and
dbus_message_iter_get_basic() and friends can return an int (due to
internal index-into-array-of-fds -> fd remapping in libdbus).
In theory int might not be the same size as any of the dbus_intNN_t
types, and anyway it's easier to see that people are getting it right
if we make it explicit.

Created attachment 53579 [details] [review]
[dbus-python] Use DBusBasicValue instead of reinventing it, if dbus is new enough

If we don't find it, continue to reinvent it, but move the reinvention
to an internal header so it's at least the same in both files that want it.

---

I'll apply this patch to dbus-python when a libdbus with public DBusBasicValue is released.

The version in the header is not directly copied from libdbus (because of the unique way in which libdbus is licensed) - it's the two unions it replaced, merged together, with the member naming reshuffled to match libdbus' API. It's pretty much equivalent to the one in libdbus, though.

For portability to AIX (if anyone cares), apparently we need to avoid struct- and union-member names like "int64" in public headers. Happily, DBusBasicValue doesn't have any of those (they're called things like "i64").

Currently proposed for 1.4 (rationale in Comment #1), but if people don't like it in 1.4 it can go in 1.5. Anyone want to review this and make binding authors' lives much easier?

ssh://people.freedesktop.org/~smcv/dbus.git 14-basic-value-11191

All three of these look pretty reasonable to me.

Fixed in git for 1.5.12, thanks for reviewing. I decided against putting this in 1.4.