[Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Stefan Weil]

On Windows, getpid() always returns an int value, but pid_t (which is
expected by the format string) is either a 32 bit or a 64 bit value.

Without a type cast (or a modified format string), the compiler prints
a warning when building for 64 bit Windows and the resulting trace_file_name
will include a wrong pid:

trace/simple.c:332:9: warning:
 format ‘%lld’ expects argument of type ‘long long int’,
 but argument 2 has type ‘int’ [-Wformat=]

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 trace/simple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/trace/simple.c b/trace/simple.c
index 11ad030..56a624c 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -329,7 +329,8 @@ bool st_set_trace_file(const char *file)
     g_free(trace_file_name);
 
     if (!file) {
-        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid());
+        /* Type cast needed for Windows where getpid() returns an int. */
+        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, (pid_t)getpid());
     } else {
         trace_file_name = g_strdup_printf("%s", file);
     }
-- 
2.1.4

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Markus Armbruster]

Stefan Weil <sw@weilnetz.de> writes:

> On Windows, getpid() always returns an int value, but pid_t (which is
> expected by the format string) is either a 32 bit or a 64 bit value.
>
> Without a type cast (or a modified format string), the compiler prints
> a warning when building for 64 bit Windows and the resulting trace_file_name
> will include a wrong pid:
>
> trace/simple.c:332:9: warning:
>  format ‘%lld’ expects argument of type ‘long long int’,
>  but argument 2 has type ‘int’ [-Wformat=]
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  trace/simple.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/trace/simple.c b/trace/simple.c
> index 11ad030..56a624c 100644
> --- a/trace/simple.c
> +++ b/trace/simple.c
> @@ -329,7 +329,8 @@ bool st_set_trace_file(const char *file)
>      g_free(trace_file_name);
>  
>      if (!file) {
> -        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid());
> +        /* Type cast needed for Windows where getpid() returns an int. */
> +        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, (pid_t)getpid());
>      } else {
>          trace_file_name = g_strdup_printf("%s", file);
>      }

First we go to the trouble of defining a platform-dependent FMT_pid, and
then we get to cast anyway.  Meh.

Can you explain why osdep.h's

    #define FMT_pid "%" PRId64

is appropriate for Windows?

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Stefan Weil]

Am 28.09.2015 um 09:00 schrieb Markus Armbruster:
> Stefan Weil <sw@weilnetz.de> writes:
>
>> On Windows, getpid() always returns an int value, but pid_t (which is
>> expected by the format string) is either a 32 bit or a 64 bit value.
>>
>> Without a type cast (or a modified format string), the compiler prints
>> a warning when building for 64 bit Windows and the resulting trace_file_name
>> will include a wrong pid:
>>
>> trace/simple.c:332:9: warning:
>>  format ‘%lld’ expects argument of type ‘long long int’,
>>  but argument 2 has type ‘int’ [-Wformat=]
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>  trace/simple.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/trace/simple.c b/trace/simple.c
>> index 11ad030..56a624c 100644
>> --- a/trace/simple.c
>> +++ b/trace/simple.c
>> @@ -329,7 +329,8 @@ bool st_set_trace_file(const char *file)
>>      g_free(trace_file_name);
>>  
>>      if (!file) {
>> -        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid());
>> +        /* Type cast needed for Windows where getpid() returns an int. */
>> +        trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, (pid_t)getpid());
>>      } else {
>>          trace_file_name = g_strdup_printf("%s", file);
>>      }
> First we go to the trouble of defining a platform-dependent FMT_pid, and
> then we get to cast anyway.  Meh.
>
> Can you explain why osdep.h's
>
>     #define FMT_pid "%" PRId64
>
> is appropriate for Windows?

Don't blame me for any strangeness which you might find in Windows. :-)

Mingw-w64 sys/types.h defines pid_t to be either an int or an __int64.
FMT_pid must match these definitions.

But getpid returns an int, not a pid_t...

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On 09/28/2015 02:26 AM, Stefan Weil wrote:

>> Can you explain why osdep.h's
>>
>>     #define FMT_pid "%" PRId64
>>
>> is appropriate for Windows?
> 
> Don't blame me for any strangeness which you might find in Windows. :-)
> 
> Mingw-w64 sys/types.h defines pid_t to be either an int or an __int64.
> FMT_pid must match these definitions.
> 
> But getpid returns an int, not a pid_t...

Can we 1) file a bug against mingw for their buggy getpid(), and 2)
write a wrapper that makes getpid() always return pid_t in the meantime?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Stefan Weil]

Am 28.09.2015 um 17:06 schrieb Eric Blake:
> On 09/28/2015 02:26 AM, Stefan Weil wrote:
>
>>> Can you explain why osdep.h's
>>>
>>>     #define FMT_pid "%" PRId64
>>>
>>> is appropriate for Windows?
>> Don't blame me for any strangeness which you might find in Windows. :-)
>>
>> Mingw-w64 sys/types.h defines pid_t to be either an int or an __int64.
>> FMT_pid must match these definitions.
>>
>> But getpid returns an int, not a pid_t...
> Can we 1) file a bug against mingw for their buggy getpid(), and 2)
> write a wrapper that makes getpid() always return pid_t in the meantime?

1) No, because MinGW and Mingw-w64 use the same declaration as MS.
They cannot change that, of course. Nor will MS correct this after many
years, for obvious reasons.

2) We could use a wrapper qemu_getpid or #define getpid() ((pid_t)getpid()).
But that would require several other code changes.

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On 09/28/2015 09:19 AM, Stefan Weil wrote:
> Am 28.09.2015 um 17:06 schrieb Eric Blake:
>> On 09/28/2015 02:26 AM, Stefan Weil wrote:
>>
>>>> Can you explain why osdep.h's
>>>>
>>>>     #define FMT_pid "%" PRId64
>>>>
>>>> is appropriate for Windows?
>>> Don't blame me for any strangeness which you might find in Windows. :-)
>>>
>>> Mingw-w64 sys/types.h defines pid_t to be either an int or an __int64.
>>> FMT_pid must match these definitions.
>>>
>>> But getpid returns an int, not a pid_t...
>> Can we 1) file a bug against mingw for their buggy getpid(), and 2)
>> write a wrapper that makes getpid() always return pid_t in the meantime?
> 
> 1) No, because MinGW and Mingw-w64 use the same declaration as MS.
> They cannot change that, of course. Nor will MS correct this after many
> years, for obvious reasons.

If getpid() always returns int, then defining pid_t to __int64 feels
wrong.  I still claim its a mingw bug to have a mismatch in types.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] trace/simple: Fix warning and wrong trace file name for MinGW

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

Am 28.09.2015 um 17:50 schrieb Eric Blake:
> On 09/28/2015 09:19 AM, Stefan Weil wrote:
>> Am 28.09.2015 um 17:06 schrieb Eric Blake:
>>> On 09/28/2015 02:26 AM, Stefan Weil wrote:
>>>
>>>>> Can you explain why osdep.h's
>>>>>
>>>>>     #define FMT_pid "%" PRId64
>>>>>
>>>>> is appropriate for Windows?
>>>> Don't blame me for any strangeness which you might find in Windows. :-)
>>>>
>>>> Mingw-w64 sys/types.h defines pid_t to be either an int or an __int64.
>>>> FMT_pid must match these definitions.
>>>>
>>>> But getpid returns an int, not a pid_t...
>>> Can we 1) file a bug against mingw for their buggy getpid(), and 2)
>>> write a wrapper that makes getpid() always return pid_t in the meantime?
>>
>> 1) No, because MinGW and Mingw-w64 use the same declaration as MS.
>> They cannot change that, of course. Nor will MS correct this after many
>> years, for obvious reasons.
> 
> If getpid() always returns int, then defining pid_t to __int64 feels
> wrong.  I still claim its a mingw bug to have a mismatch in types.
> 

I have sent this question to mingw-w64-public@lists.sourceforge.net.

Stefan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]