[Qemu-devel] [PATCH 1/3] w32: Add new directory hierarchy for MinGW extensions

[Qemu-devel] [PATCH 1/3] w32: Add new directory hierarchy for MinGW extensions

[Stefan Weil]

This is a first step which will reduce the number
of conditional compilations caused by MinGW.

Include files will be added to hosts/w32/include.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 configure        |    1 +
 hosts/w32/README |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 hosts/w32/README

diff --git a/configure b/configure
index 3036faf..47779b6 100755
--- a/configure
+++ b/configure
@@ -474,6 +474,7 @@ if test "$mingw32" = "yes" ; then
   QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
+  QEMU_CFLAGS="-I\$(SRC_PATH)/hosts/w32/include $QEMU_CFLAGS"
   LIBS="-lwinmm -lws2_32 -liberty -liphlpapi $LIBS"
   prefix="c:/Program Files/Qemu"
   mandir="\${prefix}"
diff --git a/hosts/w32/README b/hosts/w32/README
new file mode 100644
index 0000000..d5aafe9
--- /dev/null
+++ b/hosts/w32/README
@@ -0,0 +1 @@
+This directory contains extensions of MinGW which are needed by QEMU.
-- 
1.7.2.3

[Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Stefan Weil]

timersub is needed by the latest vnc code.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 hosts/w32/include/sys/time.h |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)
 create mode 100644 hosts/w32/include/sys/time.h

diff --git a/hosts/w32/include/sys/time.h b/hosts/w32/include/sys/time.h
new file mode 100644
index 0000000..94056ff
--- /dev/null
+++ b/hosts/w32/include/sys/time.h
@@ -0,0 +1,24 @@
+/*
+ * Extensions of MinGW sys/time.h
+ *
+ * Copyright (C) 2011 Stefan Weil
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include_next <sys/time.h>
+
+#ifndef timersub
+/* This is a copy from GNU C Library (GNU LGPL 2.1), sys/time.h. */
+# define timersub(a, b, result)                                               \
+  do {                                                                        \
+    (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;                             \
+    (result)->tv_usec = (a)->tv_usec - (b)->tv_usec;                          \
+    if ((result)->tv_usec < 0) {                                              \
+      --(result)->tv_sec;                                                     \
+      (result)->tv_usec += 1000000;                                           \
+    }                                                                         \
+  } while (0)
+#endif
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Blue Swirl]

On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> timersub is needed by the latest vnc code.
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  hosts/w32/include/sys/time.h |   24 ++++++++++++++++++++++++
>  1 files changed, 24 insertions(+), 0 deletions(-)
>  create mode 100644 hosts/w32/include/sys/time.h
>
> diff --git a/hosts/w32/include/sys/time.h b/hosts/w32/include/sys/time.h
> new file mode 100644
> index 0000000..94056ff
> --- /dev/null
> +++ b/hosts/w32/include/sys/time.h

Nack. The QEMU way of handling host peculiarities is to add wrappers,
for example qemu_timersub in this case, and converting all callers.

> @@ -0,0 +1,24 @@
> +/*
> + * Extensions of MinGW sys/time.h
> + *
> + * Copyright (C) 2011 Stefan Weil
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include_next <sys/time.h>

Isn't include_next an extension by GCC?

> +
> +#ifndef timersub
> +/* This is a copy from GNU C Library (GNU LGPL 2.1), sys/time.h. */
> +# define timersub(a, b, result)                                               \
> +  do {                                                                        \
> +    (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;                             \
> +    (result)->tv_usec = (a)->tv_usec - (b)->tv_usec;                          \
> +    if ((result)->tv_usec < 0) {                                              \
> +      --(result)->tv_sec;                                                     \
> +      (result)->tv_usec += 1000000;                                           \
> +    }                                                                         \
> +  } while (0)
> +#endif
> --
> 1.7.2.3
>
>
>

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Peter Maydell]

On 5 March 2011 09:34, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>> +#include_next <sys/time.h>
>
> Isn't include_next an extension by GCC?

Are gcc extensions forbidden? We already have plenty of
code that uses gcc-specific syntax or gcc extensions...

-- PMM

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Blue Swirl]

On Sat, Mar 5, 2011 at 11:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 March 2011 09:34, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>>> +#include_next <sys/time.h>
>>
>> Isn't include_next an extension by GCC?
>
> Are gcc extensions forbidden? We already have plenty of
> code that uses gcc-specific syntax or gcc extensions...

No, but relying on them is not OK. For example gcc attributes are
wrapped in macros to allow other implementations.

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Peter Maydell]

On 5 March 2011 09:48, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Mar 5, 2011 at 11:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 5 March 2011 09:34, Blue Swirl <blauwirbel@gmail.com> wrote:
>>> On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>>>> +#include_next <sys/time.h>
>>>
>>> Isn't include_next an extension by GCC?
>>
>> Are gcc extensions forbidden? We already have plenty of
>> code that uses gcc-specific syntax or gcc extensions...
>
> No, but relying on them is not OK. For example gcc attributes are
> wrapped in macros to allow other implementations.

Three random counter-examples:

vl.c:
    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */

That use of ?: is a gcc extension.

target-i386/cpu.h:
register struct CPUX86State *env asm(AREG0);

Explicit register variables are a gcc extension.

qemu-timer-common.c:
static void __attribute__((constructor)) init_get_clock(void)

gcc-specific attribute not hidden by a macro.

-- PMM

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Blue Swirl]

On Sat, Mar 5, 2011 at 11:55 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 March 2011 09:48, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Mar 5, 2011 at 11:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 5 March 2011 09:34, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>> On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>>>>> +#include_next <sys/time.h>
>>>>
>>>> Isn't include_next an extension by GCC?
>>>
>>> Are gcc extensions forbidden? We already have plenty of
>>> code that uses gcc-specific syntax or gcc extensions...
>>
>> No, but relying on them is not OK. For example gcc attributes are
>> wrapped in macros to allow other implementations.
>
> Three random counter-examples:
>
> vl.c:
>    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
>
> That use of ?: is a gcc extension.

There's no need to use that, so those should be removed.

> target-i386/cpu.h:
> register struct CPUX86State *env asm(AREG0);
>
> Explicit register variables are a gcc extension.

Is there any other way?

> qemu-timer-common.c:
> static void __attribute__((constructor)) init_get_clock(void)
>
> gcc-specific attribute not hidden by a macro.

Probably should be wrapped, but I don't think we can avoid constructors.

Re: [Qemu-devel] [PATCH 2/3] w32: Add macro timersub to sys/time.h

[Stefan Weil]

Am 05.03.2011 10:34, schrieb Blue Swirl:
> On Sun, Feb 27, 2011 at 7:52 PM, Stefan Weil <weil@mail.berlios.de> wrote:
>> timersub is needed by the latest vnc code.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>>  hosts/w32/include/sys/time.h |   24 ++++++++++++++++++++++++
>>  1 files changed, 24 insertions(+), 0 deletions(-)
>>  create mode 100644 hosts/w32/include/sys/time.h
>>
>> diff --git a/hosts/w32/include/sys/time.h b/hosts/w32/include/sys/time.h
>> new file mode 100644
>> index 0000000..94056ff
>> --- /dev/null
>> +++ b/hosts/w32/include/sys/time.h
>
> Nack. The QEMU way of handling host peculiarities is to add wrappers,
> for example qemu_timersub in this case, and converting all callers.

That's one way how things can be handled.

I know at least one QEMU maintainer who does not like
those workarounds, especially if they are needed because
of w32/w64 operating systems, so I decided to choose
a different approach here. My approach also can be easily
removed as soon as MinGW is improved (I'm sure it will
be improved).

There is another QEMU way of handling host peculiarities:
conditional compilation. Most of this kind of handling
could be eliminated by my approach.

>
>> @@ -0,0 +1,24 @@
>> +/*
>> + * Extensions of MinGW sys/time.h
>> + *
>> + * Copyright (C) 2011 Stefan Weil
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 
>> 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include_next <sys/time.h>
>
> Isn't include_next an extension by GCC?

It is, but that's not critical given the fact that QEMU does not
work without gcc.

If needed, this dependency can be fixed easily by renaming
the intermediate include files (and including the renamed files).

I don't think that those intermediate files will be needed
forever, so a clean and cheap solution like the one which
I implemented seems to be more important to me.

>
>> +
>> +#ifndef timersub
>> +/* This is a copy from GNU C Library (GNU LGPL 2.1), sys/time.h. */
>> +# define timersub(a, b, result)                                     
>>           \
>> +  do {                                                               
>>          \
>> +    (result)->tv_sec = (a)->tv_sec - (b)->tv_sec;                   
>>           \
>> +    (result)->tv_usec = (a)->tv_usec - (b)->tv_usec;                 
>>          \
>> +    if ((result)->tv_usec < 0) {                                     
>>          \
>> +      --(result)->tv_sec;                                           
>>           \
>> +      (result)->tv_usec += 1000000;                                 
>>           \
>> +    }                                                               
>>           \
>> +  } while (0)
>> +#endif
>> --
>> 1.7.2.3

By the way: it's not possible to add timersub to existing files like
oslib-win32.c or os-win32.c (another way to handle the problem)
because of incompatible licenses.

Regards,
Stefan W.

[Qemu-devel] [PATCH 3/3] osdep: Remove conditional compilation (fixes w32 compilation)

[Stefan Weil]

sys/time.h also exists for MinGW, and it is needed because
it declares struct timeval (needed by latest vnc code).

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

diff --git a/osdep.h b/osdep.h
index 27eedcf..5a667b4 100644
--- a/osdep.h
+++ b/osdep.h
@@ -8,9 +8,7 @@
 #include <sys/signal.h>
 #endif
 
-#ifndef _WIN32
 #include <sys/time.h>
-#endif
 
 #ifndef glue
 #define xglue(x, y) x ## y
-- 
1.7.2.3