qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] win32: provide gmtime_r emulation
@ 2012-09-11  7:28 Paolo Bonzini
  2012-09-11 17:15 ` Stefan Weil
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2012-09-11  7:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: sw

Windows has _s functions from C99 instead of _r functions from POSIX.
Add an emulation shim.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        Not tested beyond compilation.

 os-win32.c      | 12 ++++++++++++
 qemu-os-win32.h |  1 +
 2 file modificati, 13 inserzioni(+)

diff --git a/os-win32.c b/os-win32.c
index 13892ba..beeded2 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -55,6 +55,18 @@ int setenv(const char *name, const char *value, int overwrite)
     return result;
 }
 
+struct tm *gmtime_r(const time_t *timep, struct tm *result)
+{
+    int rc;
+    rc = _gmtime32_s(result, timep);
+    if (rc == 0) {
+        return result;
+    } else {
+        errno = rc;
+        return NULL;
+    }
+}
+
 static BOOL WINAPI qemu_ctrl_handler(DWORD type)
 {
     qemu_system_shutdown_request();
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 753679b..284cabb 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -79,6 +79,7 @@ static inline void os_set_proc_name(const char *dummy) {}
 #endif
 
 int setenv(const char *name, const char *value, int overwrite);
+struct tm *gmtime_r(const time_t *timep, struct tm *result);
 
 typedef struct {
     long tv_sec;
-- 
1.7.11.2

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

* Re: [Qemu-devel] [PATCH] win32: provide gmtime_r emulation
  2012-09-11  7:28 [Qemu-devel] [PATCH] win32: provide gmtime_r emulation Paolo Bonzini
@ 2012-09-11 17:15 ` Stefan Weil
  2012-09-11 17:57   ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Weil @ 2012-09-11 17:15 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Am 11.09.2012 09:28, schrieb Paolo Bonzini:
> Windows has _s functions from C99 instead of _r functions from POSIX.
> Add an emulation shim.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>          Not tested beyond compilation.

Hi Paolo,

latest MinGW-w64 supports _gmtime32_s, but MinGW does not,
so this solution won't link with MinGW.

With MinGW-w64, I get a compiler warning because
_gmtime32_s is not declared. The declaration is only enabled
when macro MINGW_HAS_SECURE_API is defined.

mingw-w64-crt/secapi/_gmtime32_s.cimplements _gmtime32_s
by first looking for that symbol in msvcrt.dll. Only some
newer versions of that Microsoft C Runtime library include
_gmtime32_s. Therefore the implementation includes a fallback
which uses the simple gmtime.

We should either use a wrapper based on gmtime (like the
MinGW-w64 fallback implementation).
Then a wrapper for localtime_r would also be reasonable.
I don't expect critical re-entrancy problems caused by
that pragmatic solution.

Or we use conditional compilation (#ifdef _WIN32) like it is currently
done for localtime_r in a small number of QEMU source files.

Regards,
Stefan

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

* Re: [Qemu-devel] [PATCH] win32: provide gmtime_r emulation
  2012-09-11 17:15 ` Stefan Weil
@ 2012-09-11 17:57   ` Paolo Bonzini
  2012-09-12  4:17     ` Stefan Weil
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2012-09-11 17:57 UTC (permalink / raw)
  To: Stefan Weil; +Cc: qemu-devel

Il 11/09/2012 19:15, Stefan Weil ha scritto:
> latest MinGW-w64 supports _gmtime32_s, but MinGW does not,
> so this solution won't link with MinGW.

Do we need to support anything but latest MinGW-w64?

Paolo

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

* Re: [Qemu-devel] [PATCH] win32: provide gmtime_r emulation
  2012-09-11 17:57   ` Paolo Bonzini
@ 2012-09-12  4:17     ` Stefan Weil
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Weil @ 2012-09-12  4:17 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

Am 11.09.2012 19:57, schrieb Paolo Bonzini:
> Il 11/09/2012 19:15, Stefan Weil ha scritto:
>> latest MinGW-w64 supports _gmtime32_s, but MinGW does not,
>> so this solution won't link with MinGW.
> Do we need to support anything but latest MinGW-w64?
>
> Paolo

Good question.

Pro:

* For 64 bit support, we already need MinGW-w64.
* It looks like MinGW-w64 headers are better maintained
   (printf attributes, gmtime_s, ...).
* Therefore compilations with MinGW-w64 result in much
   less compiler warnings (or even none with one of my
   patches which are still missing in git master).

Contra:

* On Windows hosts, installation of MinGW-w64 needs an
   additional installation step (MinGW-w64 on top of MinGW)
   which is not needed if someone just wants to build 32 bit
   applications.
* Cross support for MinGW-w64 is improving with newer Linux
   distributions, but still not complete (one of my hosts
   still runs Ubuntu Lucid which does not support MinGW-w64
   for 32 bit).

Maybe we should continue to support MinGW for one or two more
years until the situation stabilizes. Maybe MinGW and
MinGW-w64 will be unified again one day.

We could add gmtime_r and localtime_r implementations for
MinGW / MinGW-w64 and use the non-reentrant functions in
that implementation for the moment.

Later, we could replace that implementation by one using
the reentrant _s variants.

Or we use conditional compilation to choose whether we need
the non-reentrant or the reentrant variant. I just had a look:
__MINGW64_VERSION_MAJOR can be used to detect MinGW-w64).

Regards,

Stefan

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

end of thread, other threads:[~2012-09-12  4:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-11  7:28 [Qemu-devel] [PATCH] win32: provide gmtime_r emulation Paolo Bonzini
2012-09-11 17:15 ` Stefan Weil
2012-09-11 17:57   ` Paolo Bonzini
2012-09-12  4:17     ` Stefan Weil

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