[Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Stefan Weil]

MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
We must also fix the include statements in util/envlist.c to include
that file.

We currently don't need an implementation of strtok_r because the
code is compiled but not linked for MinGW.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 include/sysemu/os-win32.h |    2 ++
 util/envlist.c            |    7 +------
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index d0e9234..bf9edeb 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -73,6 +73,8 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result);
 #undef localtime_r
 struct tm *localtime_r(const time_t *timep, struct tm *result);
 
+char *strtok_r(char *str, const char *delim, char **saveptr);
+
 static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
 static inline void os_setup_post(void) {}
diff --git a/util/envlist.c b/util/envlist.c
index ff99fc4..ebc06cf 100644
--- a/util/envlist.c
+++ b/util/envlist.c
@@ -1,9 +1,4 @@
-#include <assert.h>
-#include <errno.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
+#include "qemu-common.h"
 #include "qemu/queue.h"
 #include "qemu/envlist.h"
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Blue Swirl]

On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil <sw@weilnetz.de> wrote:
> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
> We must also fix the include statements in util/envlist.c to include
> that file.
>
> We currently don't need an implementation of strtok_r because the
> code is compiled but not linked for MinGW.

I think it would be better to fix the build system so that unnecessary
files are not compiled.

>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  include/sysemu/os-win32.h |    2 ++
>  util/envlist.c            |    7 +------
>  2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index d0e9234..bf9edeb 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -73,6 +73,8 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result);
>  #undef localtime_r
>  struct tm *localtime_r(const time_t *timep, struct tm *result);
>
> +char *strtok_r(char *str, const char *delim, char **saveptr);
> +
>  static inline void os_setup_signal_handling(void) {}
>  static inline void os_daemonize(void) {}
>  static inline void os_setup_post(void) {}
> diff --git a/util/envlist.c b/util/envlist.c
> index ff99fc4..ebc06cf 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -1,9 +1,4 @@
> -#include <assert.h>
> -#include <errno.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <unistd.h>
> -
> +#include "qemu-common.h"
>  #include "qemu/queue.h"
>  #include "qemu/envlist.h"
>
> --
> 1.7.10.4
>
>

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Stefan Weil]

Am 17.01.2013 21:45, schrieb Blue Swirl:
> On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil<sw@weilnetz.de>  wrote:
>> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
>> We must also fix the include statements in util/envlist.c to include
>> that file.
>>
>> We currently don't need an implementation of strtok_r because the
>> code is compiled but not linked for MinGW.
>
> I think it would be better to fix the build system so that unnecessary
> files are not compiled.

That's what I suggested first, but keeping things simple is also
a good argument. Perhaps we should accept that libqemuutil.a
can contain some unnecessary files (that's the status quo!).

We also get the additional benefit of more portable code.
Even if that portability is not needed for the moment,
it might be useful later.

Stefan

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Blue Swirl]

On Thu, Jan 17, 2013 at 8:54 PM, Stefan Weil <sw@weilnetz.de> wrote:
> Am 17.01.2013 21:45, schrieb Blue Swirl:
>
>> On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil<sw@weilnetz.de>  wrote:
>>>
>>> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
>>> We must also fix the include statements in util/envlist.c to include
>>> that file.
>>>
>>> We currently don't need an implementation of strtok_r because the
>>> code is compiled but not linked for MinGW.
>>
>>
>> I think it would be better to fix the build system so that unnecessary
>> files are not compiled.
>
>
> That's what I suggested first, but keeping things simple is also
> a good argument. Perhaps we should accept that libqemuutil.a
> can contain some unnecessary files (that's the status quo!).

This could be related. I get build errors on OpenBSD:
  LINK  i386-bsd-user/qemu-i386
../libqemuutil.a(oslib-posix.o)(.text+0x540): In function `qemu_vmalloc':
/src/qemu/util/oslib-posix.c:112: multiple definition of `qemu_vmalloc'
bsd-user/mmap.o(.text+0x400):/src/qemu/bsd-user/mmap.c:78: first defined here
/usr/bin/ld: Warning: size of symbol `qemu_vmalloc' changed from 260
in bsd-user/mmap.o to 124 in ../libqemuutil.a(oslib-posix.o)

>
> We also get the additional benefit of more portable code.
> Even if that portability is not needed for the moment,
> it might be useful later.

Yes, though building as an example KVM code on Win32 may be useful one
day, that day may be distant.

>
> Stefan
>

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Paolo Bonzini]

Il 17/01/2013 22:18, Blue Swirl ha scritto:
> On Thu, Jan 17, 2013 at 8:54 PM, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 17.01.2013 21:45, schrieb Blue Swirl:
>>
>>> On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil<sw@weilnetz.de>  wrote:
>>>>
>>>> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
>>>> We must also fix the include statements in util/envlist.c to include
>>>> that file.
>>>>
>>>> We currently don't need an implementation of strtok_r because the
>>>> code is compiled but not linked for MinGW.
>>>
>>>
>>> I think it would be better to fix the build system so that unnecessary
>>> files are not compiled.
>>
>>
>> That's what I suggested first, but keeping things simple is also
>> a good argument. Perhaps we should accept that libqemuutil.a
>> can contain some unnecessary files (that's the status quo!).
> 
> This could be related. I get build errors on OpenBSD:
>   LINK  i386-bsd-user/qemu-i386
> ../libqemuutil.a(oslib-posix.o)(.text+0x540): In function `qemu_vmalloc':
> /src/qemu/util/oslib-posix.c:112: multiple definition of `qemu_vmalloc'
> bsd-user/mmap.o(.text+0x400):/src/qemu/bsd-user/mmap.c:78: first defined here
> /usr/bin/ld: Warning: size of symbol `qemu_vmalloc' changed from 260
> in bsd-user/mmap.o to 124 in ../libqemuutil.a(oslib-posix.o)

Does this fix it?

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 5d6cffc..1f4d4a3 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -74,7 +74,7 @@ void mmap_unlock(void)
 }
 #endif

-void *qemu_vmalloc(size_t size)
+static void *qemu_vmalloc(size_t size)
 {
     void *p;
     mmap_lock();

(A better change is of course to also rename qemu_vmalloc).

>>
>> We also get the additional benefit of more portable code.
>> Even if that portability is not needed for the moment,
>> it might be useful later.
> 
> Yes, though building as an example KVM code on Win32 may be useful one
> day, that day may be distant.

That's not the kind of extra code that Stefan is talking about.  It's
extra data structures (envlist.c) that are placed in a static library
but do not appear in any final build product.

Instead, "building KVM code on Win32" is in fact something we have been
doing for years, see kvm-stub.c.

Paolo

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Blue Swirl]

On Fri, Jan 18, 2013 at 8:36 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/01/2013 22:18, Blue Swirl ha scritto:
>> On Thu, Jan 17, 2013 at 8:54 PM, Stefan Weil <sw@weilnetz.de> wrote:
>>> Am 17.01.2013 21:45, schrieb Blue Swirl:
>>>
>>>> On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil<sw@weilnetz.de>  wrote:
>>>>>
>>>>> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
>>>>> We must also fix the include statements in util/envlist.c to include
>>>>> that file.
>>>>>
>>>>> We currently don't need an implementation of strtok_r because the
>>>>> code is compiled but not linked for MinGW.
>>>>
>>>>
>>>> I think it would be better to fix the build system so that unnecessary
>>>> files are not compiled.
>>>
>>>
>>> That's what I suggested first, but keeping things simple is also
>>> a good argument. Perhaps we should accept that libqemuutil.a
>>> can contain some unnecessary files (that's the status quo!).
>>
>> This could be related. I get build errors on OpenBSD:
>>   LINK  i386-bsd-user/qemu-i386
>> ../libqemuutil.a(oslib-posix.o)(.text+0x540): In function `qemu_vmalloc':
>> /src/qemu/util/oslib-posix.c:112: multiple definition of `qemu_vmalloc'
>> bsd-user/mmap.o(.text+0x400):/src/qemu/bsd-user/mmap.c:78: first defined here
>> /usr/bin/ld: Warning: size of symbol `qemu_vmalloc' changed from 260
>> in bsd-user/mmap.o to 124 in ../libqemuutil.a(oslib-posix.o)
>
> Does this fix it?
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 5d6cffc..1f4d4a3 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -74,7 +74,7 @@ void mmap_unlock(void)
>  }
>  #endif
>
> -void *qemu_vmalloc(size_t size)
> +static void *qemu_vmalloc(size_t size)
>  {
>      void *p;
>      mmap_lock();

No, it conflicts with the function prototype.

>
> (A better change is of course to also rename qemu_vmalloc).

This works.

>
>>>
>>> We also get the additional benefit of more portable code.
>>> Even if that portability is not needed for the moment,
>>> it might be useful later.
>>
>> Yes, though building as an example KVM code on Win32 may be useful one
>> day, that day may be distant.
>
> That's not the kind of extra code that Stefan is talking about.  It's
> extra data structures (envlist.c) that are placed in a static library
> but do not appear in any final build product.
>
> Instead, "building KVM code on Win32" is in fact something we have been
> doing for years, see kvm-stub.c.

I think a similar case would be to compile the non-stub versions on
Win32 also but not link them in. For example, in theory envlist.c
could use POSIX only features.

>
> Paolo

Re: [Qemu-devel] [PATCH] util: Fix compilation of envlist.c for MinGW

[Blue Swirl]

Thanks, applied.

On Wed, Jan 16, 2013 at 6:04 PM, Stefan Weil <sw@weilnetz.de> wrote:
> MinGW has no strtok_r, so we need a declaration in sysemu/os-win32.h.
> We must also fix the include statements in util/envlist.c to include
> that file.
>
> We currently don't need an implementation of strtok_r because the
> code is compiled but not linked for MinGW.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  include/sysemu/os-win32.h |    2 ++
>  util/envlist.c            |    7 +------
>  2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index d0e9234..bf9edeb 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -73,6 +73,8 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result);
>  #undef localtime_r
>  struct tm *localtime_r(const time_t *timep, struct tm *result);
>
> +char *strtok_r(char *str, const char *delim, char **saveptr);
> +
>  static inline void os_setup_signal_handling(void) {}
>  static inline void os_daemonize(void) {}
>  static inline void os_setup_post(void) {}
> diff --git a/util/envlist.c b/util/envlist.c
> index ff99fc4..ebc06cf 100644
> --- a/util/envlist.c
> +++ b/util/envlist.c
> @@ -1,9 +1,4 @@
> -#include <assert.h>
> -#include <errno.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <unistd.h>
> -
> +#include "qemu-common.h"
>  #include "qemu/queue.h"
>  #include "qemu/envlist.h"
>
> --
> 1.7.10.4
>
>