[Qemu-devel] New patches for w32, w64

[Qemu-devel] New patches for w32, w64

[Stefan Weil]

Here are some small patches needed for the w32 and w64 platforms.

[PATCH 1/3] Fix conversions from pointer to tcg_target_long
[PATCH 2/3] w64: Add definition of FMT_pid
[PATCH 3/3] w32: Fix format string regression

Kind regards,
Stefan

[Qemu-devel] [PATCH 1/3] Fix conversions from pointer to tcg_target_long

[Stefan Weil]

tcg_gen_exit_tb takes a parameter of type tcg_target_long,
so the type casts of pointer to long should be replaced by
type casts of pointer to tcg_target_long.

These changes are needed for build environments where
sizeof(long) != sizeof(void *), especially for w64.

See 4b4a72e55660abf7efe85aca78762dcfea5519ad which fixed the
same issue for the other targets.

Cc: Alexander Graf <agraf@suse.de>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 target-s390x/translate.c     |    2 +-
 target-unicore32/translate.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 77fb448..6a22fde 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -830,7 +830,7 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num, target_ulong pc)
         /* jump to same page: we can use a direct jump */
         tcg_gen_goto_tb(tb_num);
         tcg_gen_movi_i64(psw_addr, pc);
-        tcg_gen_exit_tb((long)tb + tb_num);
+        tcg_gen_exit_tb((tcg_target_long)tb + tb_num);
     } else {
         /* jump to another page: currently not optimized */
         tcg_gen_movi_i64(psw_addr, pc);
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index a15e42d..4ecb0f1 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -1056,7 +1056,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint32_t dest)
     if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) {
         tcg_gen_goto_tb(n);
         gen_set_pc_im(dest);
-        tcg_gen_exit_tb((long)tb + n);
+        tcg_gen_exit_tb((tcg_target_long)tb + n);
     } else {
         gen_set_pc_im(dest);
         tcg_gen_exit_tb(0);
-- 
1.7.2.5

[Qemu-devel] [PATCH 2/3] w64: Add definition of FMT_pid

[Stefan Weil]

For mingw-w64, pid_t is _pid_t which is __int64,
so this platform needs its own definition of FMT_pid.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 osdep.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/osdep.h b/osdep.h
index a817017..252d050 100644
--- a/osdep.h
+++ b/osdep.h
@@ -130,6 +130,8 @@ int qemu_madvise(void *addr, size_t len, int advice);
 
 #if defined(__HAIKU__) && defined(__i386__)
 #define FMT_pid "%ld"
+#elif defined(WIN64)
+#define FMT_pid "%" PRId64
 #else
 #define FMT_pid "%d"
 #endif
-- 
1.7.2.5

[Qemu-devel] [PATCH 3/3] w32: Fix format string regression

[Stefan Weil]

Commit 953ffe0f935f40c0d6061d69e76e0339393b54f8
introduced FMT_pid which is wrong for w32 and w64 getpid():
those getpid() implementations always return an int value.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 os-win32.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index b6652af..e153058 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -258,7 +258,7 @@ int qemu_create_pidfile(const char *filename)
     if (file == INVALID_HANDLE_VALUE) {
         return -1;
     }
-    len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid());
+    len = snprintf(buffer, sizeof(buffer), "%d\n", getpid());
     ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
 		      &overlap, NULL);
     if (ret == 0) {
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 2/3] w64: Add definition of FMT_pid

[Andreas Färber]

Am 15.07.2011 um 21:38 schrieb Stefan Weil:

> For mingw-w64, pid_t is _pid_t which is __int64,
> so this platform needs its own definition of FMT_pid.
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>

Ah, that's probably lld then and thus different from the existing  
definitions.

Reviewed-by: Andreas Färber <andreas.faerber@web.de>

Andreas

> ---
> osdep.h |    2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/osdep.h b/osdep.h
> index a817017..252d050 100644
> --- a/osdep.h
> +++ b/osdep.h
> @@ -130,6 +130,8 @@ int qemu_madvise(void *addr, size_t len, int  
> advice);
>
> #if defined(__HAIKU__) && defined(__i386__)
> #define FMT_pid "%ld"
> +#elif defined(WIN64)
> +#define FMT_pid "%" PRId64
> #else
> #define FMT_pid "%d"
> #endif
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 3/3] w32: Fix format string regression

[Andreas Färber]

Am 15.07.2011 um 21:38 schrieb Stefan Weil:

> Commit 953ffe0f935f40c0d6061d69e76e0339393b54f8
> introduced FMT_pid which is wrong for w32 and w64 getpid():
> those getpid() implementations always return an int value.

Where is __int64 FMT_pid used then if not for the returned pid?

Andreas

>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
> os-win32.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/os-win32.c b/os-win32.c
> index b6652af..e153058 100644
> --- a/os-win32.c
> +++ b/os-win32.c
> @@ -258,7 +258,7 @@ int qemu_create_pidfile(const char *filename)
>     if (file == INVALID_HANDLE_VALUE) {
>         return -1;
>     }
> -    len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid());
> +    len = snprintf(buffer, sizeof(buffer), "%d\n", getpid());
>     ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
> 		      &overlap, NULL);
>     if (ret == 0) {
> -- 
> 1.7.2.5
>
>

Re: [Qemu-devel] [PATCH 3/3] w32: Fix format string regression

[Blue Swirl]

On Fri, Jul 15, 2011 at 11:42 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 15.07.2011 um 21:38 schrieb Stefan Weil:
>
>> Commit 953ffe0f935f40c0d6061d69e76e0339393b54f8
>> introduced FMT_pid which is wrong for w32 and w64 getpid():
>> those getpid() implementations always return an int value.

This is not in line with Posix:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpid.html#tag_16_243

> Where is __int64 FMT_pid used then if not for the returned pid?

For shutdown_pid which is pid_t:
src/qemu/vl.c:1200:            fprintf(stderr, " from pid " FMT_pid
"\n", shutdown_pid);

> Andreas
>
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>> os-win32.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/os-win32.c b/os-win32.c
>> index b6652af..e153058 100644
>> --- a/os-win32.c
>> +++ b/os-win32.c
>> @@ -258,7 +258,7 @@ int qemu_create_pidfile(const char *filename)
>>    if (file == INVALID_HANDLE_VALUE) {
>>        return -1;
>>    }
>> -    len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid());
>> +    len = snprintf(buffer, sizeof(buffer), "%d\n", getpid());

Another way would be to cast the result of getpid to pid_t. Then if
the buggy getpid() implementations are fixed one day, there will be no
truncation.

>>    ret = WriteFileEx(file, (LPCVOID)buffer, (DWORD)len,
>>                      &overlap, NULL);
>>    if (ret == 0) {
>> --
>> 1.7.2.5
>>
>>
>
>

Re: [Qemu-devel] [PATCH 3/3] w32: Fix format string regression

[Stefan Weil]

Am 17.07.2011 11:03, schrieb Blue Swirl:
> On Fri, Jul 15, 2011 at 11:42 PM, Andreas Färber 
> <andreas.faerber@web.de> wrote:
>> Am 15.07.2011 um 21:38 schrieb Stefan Weil:
>>> Commit 953ffe0f935f40c0d6061d69e76e0339393b54f8
>>> introduced FMT_pid which is wrong for w32 and w64 getpid():
>>> those getpid() implementations always return an int value.
>
> This is not in line with Posix:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpid.html#tag_16_243

Maybe I should have added that getpid() is a function in a vendor 
specific API.
Microsoft does not claim that getpid() is in line with Posix
(they say it is a deprecated POSIX function, see
http://msdn.microsoft.com/en-us/library/ms235372%28v=vs.80%29.aspx).

Visual Studio uses this declaration:
_CRT_NONSTDC_DEPRECATE(_getpid) _CRTIMP int __cdecl getpid(void);

Mingw32 tries to be more POSIX like and uses (with pid_t = int) this 
declaration:
_CRTIMP pid_t __cdecl __MINGW_NOTHROW getpid (void);

Mingw-w64 is closer to the VS declaration:
int __cdecl getpid(void) __MINGW_ATTRIB_DEPRECATED_MSVC2005;

The replacement for getpid() is _getpid(). It also has a wrong
declaration in mingw32. A patch which simply replaces getpid by
_getpid is on my todo list, but not urgent, because the current
code works when my patch was applied.

Regards,
Stefan

Re: [Qemu-devel] [PATCH 3/3] w32: Fix format string regression

[Blue Swirl]

On Sun, Jul 17, 2011 at 9:34 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 17.07.2011 11:03, schrieb Blue Swirl:
>>
>> On Fri, Jul 15, 2011 at 11:42 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> Am 15.07.2011 um 21:38 schrieb Stefan Weil:
>>>>
>>>> Commit 953ffe0f935f40c0d6061d69e76e0339393b54f8
>>>> introduced FMT_pid which is wrong for w32 and w64 getpid():
>>>> those getpid() implementations always return an int value.
>>
>> This is not in line with Posix:
>>
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpid.html#tag_16_243
>
> Maybe I should have added that getpid() is a function in a vendor specific
> API.
> Microsoft does not claim that getpid() is in line with Posix
> (they say it is a deprecated POSIX function, see
> http://msdn.microsoft.com/en-us/library/ms235372%28v=vs.80%29.aspx).
>
> Visual Studio uses this declaration:
> _CRT_NONSTDC_DEPRECATE(_getpid) _CRTIMP int __cdecl getpid(void);
>
> Mingw32 tries to be more POSIX like and uses (with pid_t = int) this
> declaration:
> _CRTIMP pid_t __cdecl __MINGW_NOTHROW getpid (void);
>
> Mingw-w64 is closer to the VS declaration:
> int __cdecl getpid(void) __MINGW_ATTRIB_DEPRECATED_MSVC2005;
>
> The replacement for getpid() is _getpid(). It also has a wrong
> declaration in mingw32. A patch which simply replaces getpid by
> _getpid is on my todo list, but not urgent, because the current
> code works when my patch was applied.

Since for example GetProcessById uses int, it looks like pid_t is instead wrong.

Re: [Qemu-devel] [PATCH 1/3] Fix conversions from pointer to tcg_target_long

[Guan Xuetao]

It's good for unicore32 bit.

Acked-by: Guan Xuetao<gxt@mprc.pku.edu.cn>

Thanks & Regards.

Guan Xuetao

On Fri, 2011-07-15 at 21:38 +0200, Stefan Weil wrote:
> tcg_gen_exit_tb takes a parameter of type tcg_target_long,
> so the type casts of pointer to long should be replaced by
> type casts of pointer to tcg_target_long.
> 
> These changes are needed for build environments where
> sizeof(long) != sizeof(void *), especially for w64.
> 
> See 4b4a72e55660abf7efe85aca78762dcfea5519ad which fixed the
> same issue for the other targets.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  target-s390x/translate.c     |    2 +-
>  target-unicore32/translate.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-s390x/translate.c b/target-s390x/translate.c
> index 77fb448..6a22fde 100644
> --- a/target-s390x/translate.c
> +++ b/target-s390x/translate.c
> @@ -830,7 +830,7 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num, target_ulong pc)
>          /* jump to same page: we can use a direct jump */
>          tcg_gen_goto_tb(tb_num);
>          tcg_gen_movi_i64(psw_addr, pc);
> -        tcg_gen_exit_tb((long)tb + tb_num);
> +        tcg_gen_exit_tb((tcg_target_long)tb + tb_num);
>      } else {
>          /* jump to another page: currently not optimized */
>          tcg_gen_movi_i64(psw_addr, pc);
> diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
> index a15e42d..4ecb0f1 100644
> --- a/target-unicore32/translate.c
> +++ b/target-unicore32/translate.c
> @@ -1056,7 +1056,7 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint32_t dest)
>      if ((tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)) {
>          tcg_gen_goto_tb(n);
>          gen_set_pc_im(dest);
> -        tcg_gen_exit_tb((long)tb + n);
> +        tcg_gen_exit_tb((tcg_target_long)tb + n);
>      } else {
>          gen_set_pc_im(dest);
>          tcg_gen_exit_tb(0);

Re: [Qemu-devel] [PATCH 1/3] Fix conversions from pointer to tcg_target_long

[Alexander Graf]

On 15.07.2011, at 21:38, Stefan Weil wrote:

> tcg_gen_exit_tb takes a parameter of type tcg_target_long,
> so the type casts of pointer to long should be replaced by
> type casts of pointer to tcg_target_long.
> 
> These changes are needed for build environments where
> sizeof(long) != sizeof(void *), especially for w64.
> 
> See 4b4a72e55660abf7efe85aca78762dcfea5519ad which fixed the
> same issue for the other targets.

Changes the code to be aligned with the i386 target, so I'm in favor.

Acked-by: Alexander Graf <agraf@suse.de>


Alex

Re: [Qemu-devel] New patches for w32, w64

[Stefan Weil]

Am 15.07.2011 21:38, schrieb Stefan Weil:
> Here are some small patches needed for the w32 and w64 platforms.
>
> [PATCH 1/3] Fix conversions from pointer to tcg_target_long
> [PATCH 2/3] w64: Add definition of FMT_pid
> [PATCH 3/3] w32: Fix format string regression
>
> Kind regards,
> Stefan

What about these patches?

Patch 1 was acked by two developers:
     http://patchwork.ozlabs.org/patch/104889/

Patch 2 was review by Andreas:
     http://patchwork.ozlabs.org/patch/104884/

Patch 3 was discussed a lot:
     http://patchwork.ozlabs.org/patch/104885/

At least the first two patches can be applied immediately.

I also think that the third patch is correct and should be applied.
It's only MinGW related and fixes a compiler error (regression) there.

Cheers,
Stefan

Re: [Qemu-devel] New patches for w32, w64

[Blue Swirl]

On Sat, Aug 13, 2011 at 9:38 AM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 15.07.2011 21:38, schrieb Stefan Weil:
>>
>> Here are some small patches needed for the w32 and w64 platforms.
>>
>> [PATCH 1/3] Fix conversions from pointer to tcg_target_long
>> [PATCH 2/3] w64: Add definition of FMT_pid
>> [PATCH 3/3] w32: Fix format string regression
>>
>> Kind regards,
>> Stefan
>
> What about these patches?
>
> Patch 1 was acked by two developers:
>    http://patchwork.ozlabs.org/patch/104889/
>
> Patch 2 was review by Andreas:
>    http://patchwork.ozlabs.org/patch/104884/
>
> Patch 3 was discussed a lot:
>    http://patchwork.ozlabs.org/patch/104885/
>
> At least the first two patches can be applied immediately.

Yes.

> I also think that the third patch is correct and should be applied.
> It's only MinGW related and fixes a compiler error (regression) there.

The interfaces are broken in so many ways that I don't know what is
the right fix, maybe introducing qemu_getpid() which always returned
pid_t could hide this madness. Then 3/3 would not be needed.

Re: [Qemu-devel] New patches for w32, w64

[Blue Swirl]

Thanks, applied all.

On Fri, Jul 15, 2011 at 7:38 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Here are some small patches needed for the w32 and w64 platforms.
>
> [PATCH 1/3] Fix conversions from pointer to tcg_target_long
> [PATCH 2/3] w64: Add definition of FMT_pid
> [PATCH 3/3] w32: Fix format string regression
>
> Kind regards,
> Stefan
>