Bug 19195 - need _DBUS_UINT64_FORMAT, etc
need _DBUS_UINT64_FORMAT, etc
Status: RESOLVED FIXED
Product: dbus
Classification: Unclassified
Component: core
unspecified
Other All
: medium normal
Assigned To: Havoc Pennington
John (J5) Palmieri
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-19 16:39 UTC by Colin Walters
Modified: 2011-03-09 07:24 UTC (History)
0 users

See Also:


Attachments
0001-Add-DBUS_GNUC_PRINTF-checks-to-new-formatting-functi.patch (1.36 KB, patch)
2010-03-22 10:38 UTC, Colin Walters
Details | Splinter Review
0002-Add-DBUS_INT64_MODIFIER-define-turn-on-Wformat.patch (7.59 KB, patch)
2010-03-22 10:38 UTC, Colin Walters
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Walters 2008-12-19 16:39:17 UTC
Right now we warn on these constructs:

#ifdef DBUS_HAVE_INT64
              /* I think I probably mean "GNU libc printf" and not "GNUC"
               * but we'll wait until someone complains. If you hit this,
               * just turn off verbose mode as a workaround.
               */
#if __GNUC__
              _dbus_verbose (" u64: 0x%llx",
                             *(dbus_uint64_t*)&data[i-8]);

"llx" here is wrong; like all the printf formatters, it is for a variable size type (long long int, which is only defined to be *at least* 64 bits - it could be larger).

What we need are internal macros similar to GLib's G_INT64_FORMAT etc.
Comment 1 Colin Walters 2010-03-22 10:38:24 UTC
Created attachment 34329 [details] [review]
0001-Add-DBUS_GNUC_PRINTF-checks-to-new-formatting-functi.patch
Comment 2 Colin Walters 2010-03-22 10:38:36 UTC
Created attachment 34330 [details] [review]
0002-Add-DBUS_INT64_MODIFIER-define-turn-on-Wformat.patch
Comment 3 Will Thompson 2010-03-22 11:08:33 UTC
Comment on attachment 34329 [details] [review]
0001-Add-DBUS_GNUC_PRINTF-checks-to-new-formatting-functi.patch

This looks fine.
Comment 4 Will Thompson 2010-03-22 11:20:46 UTC
Comment on attachment 34330 [details] [review]
0002-Add-DBUS_INT64_MODIFIER-define-turn-on-Wformat.patch

dbus_uid_t and dbus_pid_t have format macros DBUS_UID_FORMAT and DBUS_PID_FORMAT; shouldn't they be used rather than casting uids and pids to int?

It's a little bit upsetting that dbus-monitor will silently omit values if configure couldn't figure out DBUS_INT64_PRINTF_MODIFIER. Maybe it should print "int64 (omitted, see fd.o#19195)"?
Comment 5 Simon McVittie 2011-02-28 10:09:12 UTC
Any progress on this?

(I'd personally be tempted to say "D-Bus now requires 64-bit integers, and support for them in printf" and see whether anyone reports that this breaks their OS... GLib has the same requirement, now.)
Comment 6 Simon McVittie 2011-03-09 07:24:52 UTC
See also Bug #35114.

This appears to have been fixed (with wjt's suggestions included) between 1.2.22 and 1.2.24.