Patchwork util: Fix compilation of envlist.c for MinGW

login
register
mail settings
Submitter Paolo Bonzini
Date Jan. 18, 2013, 8:36 a.m.
Message ID <50F90972.9030203@redhat.com>
Download mbox | patch
Permalink /patch/213523/
State New
Headers show

Comments

Paolo Bonzini - Jan. 18, 2013, 8:36 a.m.
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?


(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
Blue Swirl - Jan. 19, 2013, 8:59 a.m.
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

Patch

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();