Bug 61874

Summary: Windows compile warnings in dbus-launch-win.c
Product: dbus Reporter: Ralf Habacker <ralf.habacker>
Component: coreAssignee: Havoc Pennington <hp>
Status: RESOLVED FIXED QA Contact:
Severity: minor    
Priority: low    
Version: unspecified   
Hardware: Other   
OS: Windows (All)   
i915 platform: i915 features:
Attachments: Fixed compile warning
Fixed warning (update)
Fixed warning (update2)

Description Ralf Habacker 2013-03-05 21:59:45 UTC
http://lists.freedesktop.org/archives/dbus/2012-December/015378.html mentions a compiler warning in windows, which should be fixed.
Comment 1 Simon McVittie 2013-03-06 10:57:08 UTC
Sure, I'd review a patch.

Dropping the severity, though - in general mixing up %d vs. %ld is a significant bug (32- vs. 64-bit on most 64-bit Unixes), but as far as I understand it, there is no significant Windows platform where sizeof(int) != sizeof(long).
Comment 2 Ralf Habacker 2013-08-08 07:42:20 UTC
Created attachment 83812 [details] [review]
Fixed compile warning
Comment 3 Simon McVittie 2013-08-08 10:21:50 UTC
Am I right in thinking that DWORD is always "unsigned long", even on 64-bit Windows?

If it is, then surely it should be

    ..."error=%lu", GetLastError()

or something?
Comment 4 Simon McVittie 2013-08-08 10:23:23 UTC
(I wish I could tell you "use the Windows DWORD equivalent of GLib's G_SIZE_FORMAT" but it doesn't seem to have one...)
Comment 5 Ralf Habacker 2013-08-08 10:59:58 UTC
(In reply to comment #3)
> Am I right in thinking that DWORD is always "unsigned long", even on 64-bit
> Windows?
> If it is, then surely it should be
>     ..."error=%lu", GetLastError()
> or something?

seems not 

typedef unsigned __LONG32 DWORD;


/* Target specific macro replacement for type "long".  In the Windows API,
   the type long is always 32 bit, even if the target is 64 bit (LLP64).
   On 64 bit Cygwin, the type long is 64 bit (LP64).  So, to get the right
   sized definitions and declarations, all usage of type long in the Windows
   headers have to be replaced by the below defined macro __LONG32. */
#ifndef __LP64__	/* 32 bit target, 64 bit Mingw target */
#define __LONG32 long
#else			/* 64 bit Cygwin target */
#define __LONG32 int
Comment 6 Simon McVittie 2013-08-08 13:32:55 UTC
Ugh, I hadn't realised Cygwin had a different size model. In that case I'd vaguely prefer:

    "... %lu", (unsigned long) GetLastError()

since that cast will definitely not truncate...

I suppose the actual error codes can't go beyond 32-bit, so if you'd prefer

    "... %u", (unsigned) GetLastError()

that would be OK too.

Using (signed) int as in your current patch doesn't seem right to me, since DWORD is an unsigned type, but I suppose it's probably OK in practice.
Comment 7 Ralf Habacker 2013-08-12 15:38:56 UTC
Created attachment 83976 [details] [review]
Fixed warning (update)
Comment 8 Simon McVittie 2013-08-19 08:09:50 UTC
The format string for "(unsigned)" is %u, not %d.
Comment 9 Ralf Habacker 2013-08-19 08:32:03 UTC
Created attachment 84236 [details] [review]
Fixed warning (update2)
Comment 10 Simon McVittie 2013-08-19 09:47:03 UTC
Comment on attachment 84236 [details] [review]
Fixed warning (update2)

Review of attachment 84236 [details] [review]:

Ship it
Comment 11 Ralf Habacker 2013-08-19 19:36:59 UTC
(In reply to comment #10)
> Ship it

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.