Bug 92043 - [PATCH] dbus-monitor/dbus-send does not print int64 on 32 bit non-GNU systems
Summary: [PATCH] dbus-monitor/dbus-send does not print int64 on 32 bit non-GNU systems
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.10
Hardware: Other All
: medium normal
Assignee: D-Bus Maintainers
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-18 14:09 UTC by Natanael Copa
Modified: 2015-09-30 17:40 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Use C99 standard PRI*64 for printing 64 bit integers (6.33 KB, patch)
2015-09-18 14:11 UTC, Natanael Copa
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Natanael Copa 2015-09-18 14:09:32 UTC
On 32 bit systems which does not use GNU libc (like BSD and linux with musl libc systems), dbus-monitor and dbus-send will not print int64.

This is because currently, the configure script will set DBUS_INT64_PRINTF_MODIFIER only if GNU libc version bigger than 2.1 is detected and if DBUS_INT64_PRINTF_MODIFIER is unset nothing is printed.

The suggested fix is to use the standard C99 PRI*64 macros and check for windows exception.
Comment 1 Natanael Copa 2015-09-18 14:11:50 UTC
Created attachment 118345 [details] [review]
Use C99 standard PRI*64 for printing 64 bit integers

I have not tested it on a Windows system.
Comment 2 Simon McVittie 2015-09-18 17:10:16 UTC
Ralf, could you confirm whether "%I64x", etc. are correct for Windows?
Comment 3 Simon McVittie 2015-09-18 17:16:13 UTC
Comment on attachment 118345 [details] [review]
Use C99 standard PRI*64 for printing 64 bit integers

Review of attachment 118345 [details] [review]:
-----------------------------------------------------------------

This looks fine for master, if Ralf confirms that the Windows side of things is right.

Not for stable-branches; our usual policy is that portability to new platforms is not a stable-branch fix, because it tends to come with the possibility of breaking platforms that already worked. If you are distributing dbus packages for a musl-based platform, you are of course welcome to backport this change in your packaging.

::: dbus/dbus-marshal-basic.c
@@ +30,5 @@
>  #include <string.h>
>  
> +#if !defined(PRIx64) && defined(DBUS_WIN)
> +#define PRIx64 "%I64x"
> +#endif

This assumes that every non-Windows platform has the C99 PRIx64, PRIu64, PRId64, but I think that's OK until someone tells us about a non-Windows non-C99 platform that is still relevant.
Comment 4 Thiago Macieira 2015-09-23 15:03:54 UTC
Comment on attachment 118345 [details] [review]
Use C99 standard PRI*64 for printing 64 bit integers

Review of attachment 118345 [details] [review]:
-----------------------------------------------------------------

Thanks for the feature. It looks mostly good, except for the duplicated %.

Let me check what MSVC 2015 has (since it does support C99 headers), just to be on the safe side.

::: dbus/dbus-marshal-basic.c
@@ +29,5 @@
>  
>  #include <string.h>
>  
> +#if !defined(PRIx64) && defined(DBUS_WIN)
> +#define PRIx64 "%I64x"

Drop the %

::: dbus/dbus-marshal-recursive-util.c
@@ +31,5 @@
>  #include "dbus-internals.h"
>  #include <string.h>
>  
> +#if !defined(PRIx64) && defined(DBUS_WIN)
> +#define PRIx64 "%I64x"

Ditto.

::: tools/dbus-print-message.c
@@ +40,5 @@
>  #include "tool-common.h"
>  
> +#if defined(DBUS_WIN)
> +#if !defined(PRId64)
> +#define PRId64 "%I64d"

Ditto and below.
Comment 5 Thiago Macieira 2015-09-23 21:57:38 UTC
Comment on attachment 118345 [details] [review]
Use C99 standard PRI*64 for printing 64 bit integers

Review of attachment 118345 [details] [review]:
-----------------------------------------------------------------

MSVC 2013 and 2015 habe the definitions:

#define PRId64       _PFX_64 "d"
#define PRIi64       _PFX_64 "i"
#define PRIx64       _PFX_64 "x"

MSVC 2012 has no inttypes.h.
Comment 6 Simon McVittie 2015-09-30 17:40:45 UTC
(In reply to Thiago Macieira from comment #4)
> Drop the %

Fixed and pushed. I also had to add a missing #include <inttypes.h> in tools/dbus-print-message.c.

Fixed in git for 1.11.0 development branch. We don't normally merge portability enhancements into stable-branches, but it would be an appropriate change to cherry-pick if you are packaging dbus 1.10 in a non-GNU OS distribution.


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.