| Message ID | 1347348509-23900-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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;
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(+)