[Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Stefan Hajnoczi]

Hi Stefan,
I get the following compiler warning in Fedora 22
(mingw32-headers-4.0.2-1.fc22):

In file included from qemu/include/qemu-common.h:47:0,
                 from qemu/include/qemu/timer.h:5,
                 from qemu/include/sysemu/sysemu.h:8,
                 from os-win32.c:34:
qemu/include/sysemu/os-win32.h:77:12: warning: redundant redeclaration
of 'gmtime_r' [-Wredundant-decls]
 struct tm *gmtime_r(const time_t *timep, struct tm *result);
            ^
In file included from os-win32.c:30:0:
/usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note:
previous definition of 'gmtime_r' was here

QEMU has its own (non-reentrant) gmtime_r() and localtime_r()
functions on Windows.  os-win32.h redefines the functions so the
compiler is right to complain.

I thought about adding qemu_gmtime_r() and qemu_localtime_r()
functions to avoid the name clash.

Do you have any new thoughts on this commit which introduced the
os-win32.h definitions?

commit d3e8f95753114a827f9cd8e819b1d5cc8333f76b
Author: Stefan Weil <sw@weilnetz.de>
Date:   Sat Sep 22 22:26:19 2012 +0200

    w32: Add implementation of gmtime_r, localtime_r

    Those functions are missing in MinGW.

    Some versions of MinGW-w64 include defines for gmtime_r and localtime_r.
    Older versions of these macros are buggy (they return a pointer to a
    static variable), therefore we don't want them. Newer versions are
    similar to the code used here, but without the memset.

    The implementation which is used here is not strictly reentrant,
    but sufficiently good for QEMU on w32 or w64.

    Signed-off-by: Stefan Weil <sw@weilnetz.de>
    [blauwirbel@gmail.com: added comment about locking]
    Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

Re: [Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Stefan Weil]

Am 05.06.2015 um 16:38 schrieb Stefan Hajnoczi:
> Hi Stefan,
> I get the following compiler warning in Fedora 22
> (mingw32-headers-4.0.2-1.fc22):
>
> In file included from qemu/include/qemu-common.h:47:0,
>                   from qemu/include/qemu/timer.h:5,
>                   from qemu/include/sysemu/sysemu.h:8,
>                   from os-win32.c:34:
> qemu/include/sysemu/os-win32.h:77:12: warning: redundant redeclaration
> of 'gmtime_r' [-Wredundant-decls]
>   struct tm *gmtime_r(const time_t *timep, struct tm *result);
>              ^
> In file included from os-win32.c:30:0:
> /usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note:
> previous definition of 'gmtime_r' was here
>
> QEMU has its own (non-reentrant) gmtime_r() and localtime_r()
> functions on Windows.  os-win32.h redefines the functions so the
> compiler is right to complain.
>
> I thought about adding qemu_gmtime_r() and qemu_localtime_r()
> functions to avoid the name clash.
>
> Do you have any new thoughts on this commit which introduced the
> os-win32.h definitions?

The version provided by Debian Jessie (mingw-w64 3.2.0)
still uses macros to implement those functions - that's why
I don't see that compiler warnings.

I'd prefer a solution which conditionally includes the QEMU
declaration (include/sysemu/os-win32.h) and the implementation
(util/oslib-win32.c), either depending on the mingw-w64 version
or on the result of a configuration check done while running
configure.

Regards
Stefan

Re: [Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On Fri, Jun 05, 2015 at 07:18:53PM +0200, Stefan Weil wrote:
> Am 05.06.2015 um 16:38 schrieb Stefan Hajnoczi:
> >Hi Stefan,
> >I get the following compiler warning in Fedora 22
> >(mingw32-headers-4.0.2-1.fc22):
> >
> >In file included from qemu/include/qemu-common.h:47:0,
> >                  from qemu/include/qemu/timer.h:5,
> >                  from qemu/include/sysemu/sysemu.h:8,
> >                  from os-win32.c:34:
> >qemu/include/sysemu/os-win32.h:77:12: warning: redundant redeclaration
> >of 'gmtime_r' [-Wredundant-decls]
> >  struct tm *gmtime_r(const time_t *timep, struct tm *result);
> >             ^
> >In file included from os-win32.c:30:0:
> >/usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note:
> >previous definition of 'gmtime_r' was here
> >
> >QEMU has its own (non-reentrant) gmtime_r() and localtime_r()
> >functions on Windows.  os-win32.h redefines the functions so the
> >compiler is right to complain.
> >
> >I thought about adding qemu_gmtime_r() and qemu_localtime_r()
> >functions to avoid the name clash.
> >
> >Do you have any new thoughts on this commit which introduced the
> >os-win32.h definitions?
> 
> The version provided by Debian Jessie (mingw-w64 3.2.0)
> still uses macros to implement those functions - that's why
> I don't see that compiler warnings.
> 
> I'd prefer a solution which conditionally includes the QEMU
> declaration (include/sysemu/os-win32.h) and the implementation
> (util/oslib-win32.c), either depending on the mingw-w64 version
> or on the result of a configuration check done while running
> configure.

Does that mean you want:

1. gmtime() is a macro - use QEMU implementation
2. gmtime() is a function - use mingw function
3. gmtime() is undefined - use QEMU implementation

?

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Stefan Weil]

Am 08.06.2015 um 11:21 schrieb Stefan Hajnoczi:
> On Fri, Jun 05, 2015 at 07:18:53PM +0200, Stefan Weil wrote:
>>
>> The version provided by Debian Jessie (mingw-w64 3.2.0)
>> still uses macros to implement those functions - that's why
>> I don't see that compiler warnings.
>>
>> I'd prefer a solution which conditionally includes the QEMU
>> declaration (include/sysemu/os-win32.h) and the implementation
>> (util/oslib-win32.c), either depending on the mingw-w64 version
>> or on the result of a configuration check done while running
>> configure.
> Does that mean you want:
>
> 1. gmtime() is a macro - use QEMU implementation
> 2. gmtime() is a function - use mingw function

= declared via time.h and implemented in a standard library.

> 3. gmtime() is undefined - use QEMU implementation
>
> ?

Yes, exactly.

Re: [Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Michael S. Tsirkin]

On Fri, Jun 05, 2015 at 03:38:21PM +0100, Stefan Hajnoczi wrote:
> Hi Stefan,
> I get the following compiler warning in Fedora 22
> (mingw32-headers-4.0.2-1.fc22):
> 
> In file included from qemu/include/qemu-common.h:47:0,
>                  from qemu/include/qemu/timer.h:5,
>                  from qemu/include/sysemu/sysemu.h:8,
>                  from os-win32.c:34:
> qemu/include/sysemu/os-win32.h:77:12: warning: redundant redeclaration
> of 'gmtime_r' [-Wredundant-decls]
>  struct tm *gmtime_r(const time_t *timep, struct tm *result);
>             ^
> In file included from os-win32.c:30:0:
> /usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note:
> previous definition of 'gmtime_r' was here
> 
> QEMU has its own (non-reentrant) gmtime_r() and localtime_r()
> functions on Windows.  os-win32.h redefines the functions so the
> compiler is right to complain.
> 
> I thought about adding qemu_gmtime_r() and qemu_localtime_r()
> functions to avoid the name clash.
> 
> Do you have any new thoughts on this commit which introduced the
> os-win32.h definitions?
> 
> commit d3e8f95753114a827f9cd8e819b1d5cc8333f76b
> Author: Stefan Weil <sw@weilnetz.de>
> Date:   Sat Sep 22 22:26:19 2012 +0200
> 
>     w32: Add implementation of gmtime_r, localtime_r
> 
>     Those functions are missing in MinGW.
> 
>     Some versions of MinGW-w64 include defines for gmtime_r and localtime_r.
>     Older versions of these macros are buggy (they return a pointer to a
>     static variable), therefore we don't want them. Newer versions are
>     similar to the code used here, but without the memset.
> 
>     The implementation which is used here is not strictly reentrant,
>     but sufficiently good for QEMU on w32 or w64.
> 
>     Signed-off-by: Stefan Weil <sw@weilnetz.de>
>     [blauwirbel@gmail.com: added comment about locking]
>     Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

I tried this: it compiles, but then fails to link.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>


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)
 {

Re: [Qemu-devel] Redundant redeclaration of 'gmtime_r' with mingw64

[Daniel P. Berrange]

On Tue, Sep 22, 2015 at 04:11:34PM +0300, Michael S. Tsirkin wrote:
> On Fri, Jun 05, 2015 at 03:38:21PM +0100, Stefan Hajnoczi wrote:
> > Hi Stefan,
> > I get the following compiler warning in Fedora 22
> > (mingw32-headers-4.0.2-1.fc22):
> > 
> > In file included from qemu/include/qemu-common.h:47:0,
> >                  from qemu/include/qemu/timer.h:5,
> >                  from qemu/include/sysemu/sysemu.h:8,
> >                  from os-win32.c:34:
> > qemu/include/sysemu/os-win32.h:77:12: warning: redundant redeclaration
> > of 'gmtime_r' [-Wredundant-decls]
> >  struct tm *gmtime_r(const time_t *timep, struct tm *result);
> >             ^
> > In file included from os-win32.c:30:0:
> > /usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note:
> > previous definition of 'gmtime_r' was here
> > 
> > QEMU has its own (non-reentrant) gmtime_r() and localtime_r()
> > functions on Windows.  os-win32.h redefines the functions so the
> > compiler is right to complain.
> > 
> > I thought about adding qemu_gmtime_r() and qemu_localtime_r()
> > functions to avoid the name clash.
> > 
> > Do you have any new thoughts on this commit which introduced the
> > os-win32.h definitions?
> > 
> > commit d3e8f95753114a827f9cd8e819b1d5cc8333f76b
> > Author: Stefan Weil <sw@weilnetz.de>
> > Date:   Sat Sep 22 22:26:19 2012 +0200
> > 
> >     w32: Add implementation of gmtime_r, localtime_r
> > 
> >     Those functions are missing in MinGW.
> > 
> >     Some versions of MinGW-w64 include defines for gmtime_r and localtime_r.
> >     Older versions of these macros are buggy (they return a pointer to a
> >     static variable), therefore we don't want them. Newer versions are
> >     similar to the code used here, but without the memset.
> > 
> >     The implementation which is used here is not strictly reentrant,
> >     but sufficiently good for QEMU on w32 or w64.
> > 
> >     Signed-off-by: Stefan Weil <sw@weilnetz.de>
> >     [blauwirbel@gmail.com: added comment about locking]
> >     Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> 
> I tried this: it compiles, but then fails to link.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

I've had a more complete patch posted several times now, still waiting
to be picked up...

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 :|