qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] mingw: fix build on Fedora 22
@ 2015-09-22 13:38 Michael S. Tsirkin
  2015-09-22 13:48 ` Daniel P. Berrange
  0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2015-09-22 13:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Igor Mammedov, Stefan Hajnoczi,
	Stefan Hajnoczi, Stefan Weil, Denis V. Lunev

mingw on Fedora 22 replaces localtime_r and gmtime_r
macros with posix compliant functions in time.h.

These conflict with QEMU supplied ones.

Detect this and avoid overriding them.

We also need to define POSIX_C_SOURCE before including
time.h for the first time, to make sure these
are available to all users.

Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/qemu/osdep.h      | 1 +
 include/sysemu/os-win32.h | 6 ++++++
 util/oslib-win32.c        | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ab3c876..9920ac3 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -25,6 +25,7 @@
 #ifndef QEMU_OSDEP_H
 #define QEMU_OSDEP_H
 
+#define _POSIX_C_SOURCE 200809L
 #include "config-host.h"
 #include "qemu/compiler.h"
 #include <stdarg.h>
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 706d85a..74230e7 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -72,11 +72,17 @@
 #define sigsetjmp(env, savemask) setjmp(env)
 #define siglongjmp(env, val) longjmp(env, val)
 
+#ifdef gmtime_r
 /* Missing POSIX functions. Don't use MinGW-w64 macros. */
 #undef gmtime_r
+#define QEMU_NEED_GMTIME_R
 struct tm *gmtime_r(const time_t *timep, struct tm *result);
+#endif
+#ifdef localtime_r
 #undef localtime_r
+#define QEMU_NEED_LOCALTIME_R
 struct tm *localtime_r(const time_t *timep, struct tm *result);
+#endif
 
 
 static inline void os_setup_signal_handling(void) {}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 730a670..abd710a 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -95,6 +95,7 @@ void qemu_anon_ram_free(void *ptr, size_t size)
     }
 }
 
+#ifdef QEMU_NEED_GMTIME_R
 /* FIXME: add proper locking */
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
@@ -106,7 +107,9 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result)
     }
     return p;
 }
+#endif
 
+#ifdef QEMU_NEED_LOCALTIME_R
 /* FIXME: add proper locking */
 struct tm *localtime_r(const time_t *timep, struct tm *result)
 {
@@ -118,6 +121,7 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
     }
     return p;
 }
+#endif
 
 void qemu_set_block(int fd)
 {
-- 
MST

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] mingw: fix build on Fedora 22
  2015-09-22 13:38 [Qemu-devel] [PATCH] mingw: fix build on Fedora 22 Michael S. Tsirkin
@ 2015-09-22 13:48 ` Daniel P. Berrange
  2015-09-22 14:08   ` Michael S. Tsirkin
  2015-09-22 14:09   ` Denis V. Lunev
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel P. Berrange @ 2015-09-22 13:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Kevin Wolf, Peter Maydell, Igor Mammedov, Stefan Hajnoczi,
	qemu-devel, Stefan Hajnoczi, Stefan Weil, Denis V. Lunev

On Tue, Sep 22, 2015 at 04:38:31PM +0300, Michael S. Tsirkin wrote:
> mingw on Fedora 22 replaces localtime_r and gmtime_r
> macros with posix compliant functions in time.h.
> 
> These conflict with QEMU supplied ones.
> 
> Detect this and avoid overriding them.
> 
> We also need to define POSIX_C_SOURCE before including
> time.h for the first time, to make sure these
> are available to all users.
> 
> Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> Cc: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/qemu/osdep.h      | 1 +
>  include/sysemu/os-win32.h | 6 ++++++
>  util/oslib-win32.c        | 4 ++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ab3c876..9920ac3 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -25,6 +25,7 @@
>  #ifndef QEMU_OSDEP_H
>  #define QEMU_OSDEP_H
>  
> +#define _POSIX_C_SOURCE 200809L
>  #include "config-host.h"
>  #include "qemu/compiler.h"
>  #include <stdarg.h>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index 706d85a..74230e7 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -72,11 +72,17 @@
>  #define sigsetjmp(env, savemask) setjmp(env)
>  #define siglongjmp(env, val) longjmp(env, val)
>  
> +#ifdef gmtime_r
>  /* Missing POSIX functions. Don't use MinGW-w64 macros. */
>  #undef gmtime_r
> +#define QEMU_NEED_GMTIME_R
>  struct tm *gmtime_r(const time_t *timep, struct tm *result);
> +#endif

I don't think this is right. There are three possibilities
with mingw

 - No gmtime_r at all  - need replacement
 - gmtime_r defined as a macro - need replacement
 - gmtime_r defined as a function - nothing needed

With this change of yours, if gmtime_r is defined as a macro
QEMU will provide its own replacement. If gmtime_r is not
defined at all, then QEMU doesn't provide its replacement
which is wrong. I believe the change I previously proposed
works in all three cases

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01926.html

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] mingw: fix build on Fedora 22
  2015-09-22 13:48 ` Daniel P. Berrange
@ 2015-09-22 14:08   ` Michael S. Tsirkin
  2015-09-22 14:14     ` Daniel P. Berrange
  2015-09-22 14:09   ` Denis V. Lunev
  1 sibling, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2015-09-22 14:08 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Kevin Wolf, Peter Maydell, Igor Mammedov, Stefan Hajnoczi,
	qemu-devel, Stefan Hajnoczi, Stefan Weil, Denis V. Lunev

On Tue, Sep 22, 2015 at 02:48:04PM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 22, 2015 at 04:38:31PM +0300, Michael S. Tsirkin wrote:
> > mingw on Fedora 22 replaces localtime_r and gmtime_r
> > macros with posix compliant functions in time.h.
> > 
> > These conflict with QEMU supplied ones.
> > 
> > Detect this and avoid overriding them.
> > 
> > We also need to define POSIX_C_SOURCE before including
> > time.h for the first time, to make sure these
> > are available to all users.
> > 
> > Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
> > Cc: Stefan Weil <sw@weilnetz.de>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/qemu/osdep.h      | 1 +
> >  include/sysemu/os-win32.h | 6 ++++++
> >  util/oslib-win32.c        | 4 ++++
> >  3 files changed, 11 insertions(+)
> > 
> > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> > index ab3c876..9920ac3 100644
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -25,6 +25,7 @@
> >  #ifndef QEMU_OSDEP_H
> >  #define QEMU_OSDEP_H
> >  
> > +#define _POSIX_C_SOURCE 200809L
> >  #include "config-host.h"
> >  #include "qemu/compiler.h"
> >  #include <stdarg.h>
> > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > index 706d85a..74230e7 100644
> > --- a/include/sysemu/os-win32.h
> > +++ b/include/sysemu/os-win32.h
> > @@ -72,11 +72,17 @@
> >  #define sigsetjmp(env, savemask) setjmp(env)
> >  #define siglongjmp(env, val) longjmp(env, val)
> >  
> > +#ifdef gmtime_r
> >  /* Missing POSIX functions. Don't use MinGW-w64 macros. */
> >  #undef gmtime_r
> > +#define QEMU_NEED_GMTIME_R
> >  struct tm *gmtime_r(const time_t *timep, struct tm *result);
> > +#endif
> 
> I don't think this is right. There are three possibilities
> with mingw
> 
>  - No gmtime_r at all  - need replacement
>  - gmtime_r defined as a macro - need replacement
>  - gmtime_r defined as a function - nothing needed
> 
> With this change of yours, if gmtime_r is defined as a macro
> QEMU will provide its own replacement. If gmtime_r is not
> defined at all, then QEMU doesn't provide its replacement
> which is wrong. I believe the change I previously proposed
> works in all three cases
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01926.html
> 
> Regards,
> Daniel

Good point.
Can you bounce me your patch pls?

> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] mingw: fix build on Fedora 22
  2015-09-22 13:48 ` Daniel P. Berrange
  2015-09-22 14:08   ` Michael S. Tsirkin
@ 2015-09-22 14:09   ` Denis V. Lunev
  1 sibling, 0 replies; 5+ messages in thread
From: Denis V. Lunev @ 2015-09-22 14:09 UTC (permalink / raw)
  To: Daniel P. Berrange, Michael S. Tsirkin
  Cc: Kevin Wolf, Peter Maydell, Stefan Hajnoczi, qemu-devel,
	Stefan Hajnoczi, Stefan Weil, Igor Mammedov

On 09/22/2015 04:48 PM, Daniel P. Berrange wrote:
> On Tue, Sep 22, 2015 at 04:38:31PM +0300, Michael S. Tsirkin wrote:
>> mingw on Fedora 22 replaces localtime_r and gmtime_r
>> macros with posix compliant functions in time.h.
>>
>> These conflict with QEMU supplied ones.
>>
>> Detect this and avoid overriding them.
>>
>> We also need to define POSIX_C_SOURCE before including
>> time.h for the first time, to make sure these
>> are available to all users.
>>
>> Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
>> Cc: Stefan Weil <sw@weilnetz.de>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   include/qemu/osdep.h      | 1 +
>>   include/sysemu/os-win32.h | 6 ++++++
>>   util/oslib-win32.c        | 4 ++++
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>> index ab3c876..9920ac3 100644
>> --- a/include/qemu/osdep.h
>> +++ b/include/qemu/osdep.h
>> @@ -25,6 +25,7 @@
>>   #ifndef QEMU_OSDEP_H
>>   #define QEMU_OSDEP_H
>>   
>> +#define _POSIX_C_SOURCE 200809L
>>   #include "config-host.h"
>>   #include "qemu/compiler.h"
>>   #include <stdarg.h>
>> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
>> index 706d85a..74230e7 100644
>> --- a/include/sysemu/os-win32.h
>> +++ b/include/sysemu/os-win32.h
>> @@ -72,11 +72,17 @@
>>   #define sigsetjmp(env, savemask) setjmp(env)
>>   #define siglongjmp(env, val) longjmp(env, val)
>>   
>> +#ifdef gmtime_r
>>   /* Missing POSIX functions. Don't use MinGW-w64 macros. */
>>   #undef gmtime_r
>> +#define QEMU_NEED_GMTIME_R
>>   struct tm *gmtime_r(const time_t *timep, struct tm *result);
>> +#endif
> I don't think this is right. There are three possibilities
> with mingw
>
>   - No gmtime_r at all  - need replacement
>   - gmtime_r defined as a macro - need replacement
>   - gmtime_r defined as a function - nothing needed
>
> With this change of yours, if gmtime_r is defined as a macro
> QEMU will provide its own replacement. If gmtime_r is not
> defined at all, then QEMU doesn't provide its replacement
> which is wrong. I believe the change I previously proposed
> works in all three cases
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01926.html
indeed, this looks better to me.

Den

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] mingw: fix build on Fedora 22
  2015-09-22 14:08   ` Michael S. Tsirkin
@ 2015-09-22 14:14     ` Daniel P. Berrange
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrange @ 2015-09-22 14:14 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Kevin Wolf, Peter Maydell, Igor Mammedov, Stefan Hajnoczi,
	qemu-devel, Stefan Hajnoczi, Stefan Weil, Denis V. Lunev

On Tue, Sep 22, 2015 at 05:08:44PM +0300, Michael S. Tsirkin wrote:
> On Tue, Sep 22, 2015 at 02:48:04PM +0100, Daniel P. Berrange wrote:
> > 
> > I don't think this is right. There are three possibilities
> > with mingw
> > 
> >  - No gmtime_r at all  - need replacement
> >  - gmtime_r defined as a macro - need replacement
> >  - gmtime_r defined as a function - nothing needed
> > 
> > With this change of yours, if gmtime_r is defined as a macro
> > QEMU will provide its own replacement. If gmtime_r is not
> > defined at all, then QEMU doesn't provide its replacement
> > which is wrong. I believe the change I previously proposed
> > works in all three cases
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01926.html
> > 
> Good point.
> Can you bounce me your patch pls?

I just sent it as a REPOST, with everyone on this thread CC'd.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-09-22 14:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-22 13:38 [Qemu-devel] [PATCH] mingw: fix build on Fedora 22 Michael S. Tsirkin
2015-09-22 13:48 ` Daniel P. Berrange
2015-09-22 14:08   ` Michael S. Tsirkin
2015-09-22 14:14     ` Daniel P. Berrange
2015-09-22 14:09   ` Denis V. Lunev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).