Bug 61874 - Windows compile warnings in dbus-launch-win.c
Summary: Windows compile warnings in dbus-launch-win.c
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: low minor
Assignee: Havoc Pennington
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-05 21:59 UTC by Ralf Habacker
Modified: 2013-08-19 19:36 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Fixed compile warning (771 bytes, patch)
2013-08-08 07:42 UTC, Ralf Habacker
Details | Splinter Review
Fixed warning (update) (777 bytes, patch)
2013-08-12 15:38 UTC, Ralf Habacker
Details | Splinter Review
Fixed warning (update2) (777 bytes, patch)
2013-08-19 08:32 UTC, Ralf Habacker
Details | Splinter Review

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 

windef.h 
... 
typedef unsigned __LONG32 DWORD;

_mingw.h

/* 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
#endif
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
committed


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.