> #ifdef DBUS_HAVE_INT64 > dbus_uint64_t value; > int type; > dbus_message_iter_get_basic (&read_iter, &value); > type = dbus_message_iter_get_arg_type (&read_iter); > dbus_message_iter_append_basic (&write_iter, type, &value); > #endif > > You can skip the DBUS_HAVE_INT64 conditional unless you care about some sort > of really obscure platform. If you do know about such a platform and want your > code to work on it, create a struct that occupies at least 8 bytes. > dbus_uint64_t is just one example of a type that's large enough to hold any > possible value. The #ifdef makes no sense, since you clearly wouldn't conditionalize the whole thing on any platform. The right thing to do if you aren't catering to an obscure platform is to use dbus_uint64_t value uncondtionalized so it *breaks* in the compiler if you are on an obscure platform and the code will get fixed. I think that's the right thing to do in the docs, and skip the irrelevant chatter. 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 :-)
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.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.