Re: [Qemu-devel] [PATCH 1/2] oslib-win32: add lock for gmtime_r()

[Wenchao Xia]

于 2013-1-9 17:47, Paolo Bonzini 写道:
> Il 09/01/2013 07:18, Wenchao Xia ha scritto:
> >    Also changed the caller of gmtime() to gmtime_r().
>
> Should be a separate patch.
>
> Also, what is the reason to add this lock?  They are already protected
> by the BQL.
>
> Paolo
 localtime_r() will be used in internal snapshot. I am not sure if it is
already protected by BQL, if yes I think this can be dropped but
add comments on API "need external lock".
  But with this lock API calling them can be tagged as "thread safe",
which would be much nicer.

> > Signed-off-by: Wenchao Xia <address@hidden>
> > ---
> >   oslib-win32.c |    6 +++++-
> >   vl.c          |    4 ++--
> >   2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/oslib-win32.c b/oslib-win32.c
> > index e7e283e..74e0c52 100644
> > --- a/oslib-win32.c
> > +++ b/oslib-win32.c
> > @@ -32,6 +32,8 @@
> >   #include "trace.h"
> >   #include "qemu/sockets.h"
> >
> > +static GStaticMutex time_lock = G_STATIC_MUTEX_INIT;
> > +
> >   void *qemu_oom_check(void *ptr)
> >   {
> >       if (ptr == NULL) {
> > @@ -74,15 +76,17 @@ void qemu_vfree(void *ptr)
> >       VirtualFree(ptr, 0, MEM_RELEASE);
> >   }
> >
> > -/* FIXME: add proper locking */
> > +/* FIXME: add proper locking in MinGW */
> >   struct tm *gmtime_r(const time_t *timep, struct tm *result)
> >   {
> > +    g_static_mutex_lock(&time_lock);
> >       struct tm *p = gmtime(timep);
> >       memset(result, 0, sizeof(*result));
> >       if (p) {
> >           *result = *p;
> >           p = result;
> >       }
> > +    g_static_mutex_unlock(&time_lock);
> >       return p;
> >   }
> >
> > diff --git a/vl.c b/vl.c
> > index 79e5122..8d7864c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -454,12 +454,12 @@ void qemu_get_timedate(struct tm *tm, int offset)
> >       ti += offset;
> >       if (rtc_date_offset == -1) {
> >           if (rtc_utc)
> > -            ret = gmtime(&ti);
> > +            ret = gmtime_r(&ti, tm);
> >           else
> >               ret = localtime(&ti);
> >       } else {
> >           ti -= rtc_date_offset;
> > -        ret = gmtime(&ti);
> > +        ret = gmtime_r(&ti, tm);
> >       }
> >
> >       memcpy(tm, ret, sizeof(struct tm));


--
Best Regards

Wenchao Xia