qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r
@ 2012-09-22 20:26 Stefan Weil
  2012-09-23 15:59 ` Blue Swirl
  2012-09-23 17:10 ` Blue Swirl
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Weil @ 2012-09-22 20:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Stefan Weil

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>
---
 oslib-win32.c   |   22 ++++++++++++++++++++++
 qemu-os-win32.h |    6 ++++++
 2 files changed, 28 insertions(+)

diff --git a/oslib-win32.c b/oslib-win32.c
index ffbc6d0..2acbf9b 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -74,6 +74,28 @@ void qemu_vfree(void *ptr)
     VirtualFree(ptr, 0, MEM_RELEASE);
 }
 
+struct tm *gmtime_r(const time_t *timep, struct tm *result)
+{
+    struct tm *p = gmtime(timep);
+    memset(result, 0, sizeof(*result));
+    if (p) {
+        *result = *p;
+        p = result;
+    }
+    return p;
+}
+
+struct tm *localtime_r(const time_t *timep, struct tm *result)
+{
+    struct tm *p = localtime(timep);
+    memset(result, 0, sizeof(*result));
+    if (p) {
+        *result = *p;
+        p = result;
+    }
+    return p;
+}
+
 void socket_set_block(int fd)
 {
     unsigned long opt = 0;
diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 753679b..3b5a35b 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -68,6 +68,12 @@
 /* Declaration of ffs() is missing in MinGW's strings.h. */
 int ffs(int i);
 
+/* Missing POSIX functions. Don't use MinGW-w64 macros. */
+#undef gmtime_r
+struct tm *gmtime_r(const time_t *timep, struct tm *result);
+#undef localtime_r
+struct tm *localtime_r(const time_t *timep, struct tm *result);
+
 static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
 static inline void os_setup_post(void) {}
-- 
1.7.10

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

* Re: [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r
  2012-09-22 20:26 [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r Stefan Weil
@ 2012-09-23 15:59 ` Blue Swirl
  2012-09-23 16:27   ` Stefan Weil
  2012-09-23 17:10 ` Blue Swirl
  1 sibling, 1 reply; 5+ messages in thread
From: Blue Swirl @ 2012-09-23 15:59 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Sat, Sep 22, 2012 at 8:26 PM, Stefan Weil <sw@weilnetz.de> wrote:
> 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.

For now, but nothing shows that there is a problem. Adding a few
simple locks shouldn't be difficult, or FIXME/XXX comment otherwise.

>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>  oslib-win32.c   |   22 ++++++++++++++++++++++
>  qemu-os-win32.h |    6 ++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/oslib-win32.c b/oslib-win32.c
> index ffbc6d0..2acbf9b 100644
> --- a/oslib-win32.c
> +++ b/oslib-win32.c
> @@ -74,6 +74,28 @@ void qemu_vfree(void *ptr)
>      VirtualFree(ptr, 0, MEM_RELEASE);
>  }
>
> +struct tm *gmtime_r(const time_t *timep, struct tm *result)
> +{
> +    struct tm *p = gmtime(timep);
> +    memset(result, 0, sizeof(*result));
> +    if (p) {
> +        *result = *p;
> +        p = result;
> +    }
> +    return p;
> +}
> +
> +struct tm *localtime_r(const time_t *timep, struct tm *result)
> +{
> +    struct tm *p = localtime(timep);
> +    memset(result, 0, sizeof(*result));
> +    if (p) {
> +        *result = *p;
> +        p = result;
> +    }
> +    return p;
> +}
> +
>  void socket_set_block(int fd)
>  {
>      unsigned long opt = 0;
> diff --git a/qemu-os-win32.h b/qemu-os-win32.h
> index 753679b..3b5a35b 100644
> --- a/qemu-os-win32.h
> +++ b/qemu-os-win32.h
> @@ -68,6 +68,12 @@
>  /* Declaration of ffs() is missing in MinGW's strings.h. */
>  int ffs(int i);
>
> +/* Missing POSIX functions. Don't use MinGW-w64 macros. */
> +#undef gmtime_r
> +struct tm *gmtime_r(const time_t *timep, struct tm *result);
> +#undef localtime_r
> +struct tm *localtime_r(const time_t *timep, struct tm *result);
> +
>  static inline void os_setup_signal_handling(void) {}
>  static inline void os_daemonize(void) {}
>  static inline void os_setup_post(void) {}
> --
> 1.7.10
>
>

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

* Re: [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r
  2012-09-23 15:59 ` Blue Swirl
@ 2012-09-23 16:27   ` Stefan Weil
  2012-09-23 16:37     ` Blue Swirl
  0 siblings, 1 reply; 5+ messages in thread
From: Stefan Weil @ 2012-09-23 16:27 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Paolo Bonzini, qemu-devel

> On Sat, Sep 22, 2012 at 8:26 PM, Stefan Weil <sw@weilnetz.de> wrote:
>> 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.
>
> For now, but nothing shows that there is a problem. Adding a few
> simple locks shouldn't be difficult, or FIXME/XXX comment otherwise.
>

Does (or will) QEMU support preemptive scheduling of threads?
If not, there is no reentrancy problem because gmtime / localtime
don't trigger scheduling.

Of course adding comments is always a good idea. Feel free to add one
or wait until I send a v2 patch.

Regards

Stefan

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

* Re: [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r
  2012-09-23 16:27   ` Stefan Weil
@ 2012-09-23 16:37     ` Blue Swirl
  0 siblings, 0 replies; 5+ messages in thread
From: Blue Swirl @ 2012-09-23 16:37 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Sun, Sep 23, 2012 at 4:27 PM, Stefan Weil <sw@weilnetz.de> wrote:
>> On Sat, Sep 22, 2012 at 8:26 PM, Stefan Weil <sw@weilnetz.de> wrote:
>>> 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.
>>
>> For now, but nothing shows that there is a problem. Adding a few
>> simple locks shouldn't be difficult, or FIXME/XXX comment otherwise.
>>
>
> Does (or will) QEMU support preemptive scheduling of threads?
> If not, there is no reentrancy problem because gmtime / localtime
> don't trigger scheduling.

Right, but it looks like the plan is to use thread safe versions. If
not, reverting the original patch that introduced _r versions would
have been OK.

>
> Of course adding comments is always a good idea. Feel free to add one
> or wait until I send a v2 patch.

OK, I'll add a comment.

>
> Regards
>
> Stefan
>
>

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

* Re: [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r
  2012-09-22 20:26 [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r Stefan Weil
  2012-09-23 15:59 ` Blue Swirl
@ 2012-09-23 17:10 ` Blue Swirl
  1 sibling, 0 replies; 5+ messages in thread
From: Blue Swirl @ 2012-09-23 17:10 UTC (permalink / raw)
  To: Stefan Weil; +Cc: Paolo Bonzini, qemu-devel

On Sat, Sep 22, 2012 at 8:26 PM, Stefan Weil <sw@weilnetz.de> wrote:
> 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>

Thanks, applied with comment.

> ---
>  oslib-win32.c   |   22 ++++++++++++++++++++++
>  qemu-os-win32.h |    6 ++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/oslib-win32.c b/oslib-win32.c
> index ffbc6d0..2acbf9b 100644
> --- a/oslib-win32.c
> +++ b/oslib-win32.c
> @@ -74,6 +74,28 @@ void qemu_vfree(void *ptr)
>      VirtualFree(ptr, 0, MEM_RELEASE);
>  }
>
> +struct tm *gmtime_r(const time_t *timep, struct tm *result)
> +{
> +    struct tm *p = gmtime(timep);
> +    memset(result, 0, sizeof(*result));
> +    if (p) {
> +        *result = *p;
> +        p = result;
> +    }
> +    return p;
> +}
> +
> +struct tm *localtime_r(const time_t *timep, struct tm *result)
> +{
> +    struct tm *p = localtime(timep);
> +    memset(result, 0, sizeof(*result));
> +    if (p) {
> +        *result = *p;
> +        p = result;
> +    }
> +    return p;
> +}
> +
>  void socket_set_block(int fd)
>  {
>      unsigned long opt = 0;
> diff --git a/qemu-os-win32.h b/qemu-os-win32.h
> index 753679b..3b5a35b 100644
> --- a/qemu-os-win32.h
> +++ b/qemu-os-win32.h
> @@ -68,6 +68,12 @@
>  /* Declaration of ffs() is missing in MinGW's strings.h. */
>  int ffs(int i);
>
> +/* Missing POSIX functions. Don't use MinGW-w64 macros. */
> +#undef gmtime_r
> +struct tm *gmtime_r(const time_t *timep, struct tm *result);
> +#undef localtime_r
> +struct tm *localtime_r(const time_t *timep, struct tm *result);
> +
>  static inline void os_setup_signal_handling(void) {}
>  static inline void os_daemonize(void) {}
>  static inline void os_setup_post(void) {}
> --
> 1.7.10
>
>

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

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

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-22 20:26 [Qemu-devel] [PATCH] w32: Add implementation of gmtime_r, localtime_r Stefan Weil
2012-09-23 15:59 ` Blue Swirl
2012-09-23 16:27   ` Stefan Weil
2012-09-23 16:37     ` Blue Swirl
2012-09-23 17:10 ` Blue Swirl

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