Patchwork win32: provide gmtime_r emulation

login
register
mail settings
Submitter Paolo Bonzini
Date Sept. 11, 2012, 7:28 a.m.
Message ID <1347348509-23900-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/183038/
State New
Headers show

Comments

Paolo Bonzini - Sept. 11, 2012, 7:28 a.m.
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(+)
Stefan Weil - Sept. 11, 2012, 5:15 p.m.
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
Paolo Bonzini - Sept. 11, 2012, 5:57 p.m.
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
Stefan Weil - Sept. 12, 2012, 4:17 a.m.
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

Patch

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;