| Message ID | 1357712290-16496-1-git-send-email-xiawenc@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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 > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > --- > 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)); >
> Il 09/01/2013 07:18, Wenchao Xia ha scritto: >> Also changed the caller of gmtime() to gmtime_r(). > > Should be a separate patch. There is already a pending patch which replaces gmtime, localtime: http://patchwork.ozlabs.org/patch/210250/ Stefan
于 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 <xiawenc@linux.vnet.ibm.com> >> --- >> 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)); >> >
于 2013-1-10 1:06, Stefan Weil 写道: >> Il 09/01/2013 07:18, Wenchao Xia ha scritto: >>> Also changed the caller of gmtime() to gmtime_r(). >> >> Should be a separate patch. > > There is already a pending patch which replaces gmtime, localtime: > > http://patchwork.ozlabs.org/patch/210250/ > > Stefan > > Should other calling to localtime be changed to localtime_r() also?
>>> Il 09/01/2013 07:18, Wenchao Xia ha scritto: >>>> Also changed the caller of gmtime() to gmtime_r(). >>> >>> Should be a separate patch. >> >> There is already a pending patch which replaces gmtime, localtime: >> >> http://patchwork.ozlabs.org/patch/210250/ >> >> Stefan >> >> > Should other calling to localtime be changed to localtime_r() also? > > -- > Best Regards > > Wenchao Xia With http://patchwork.ozlabs.org/patch/210240/ and http://patchwork.ozlabs.org/patch/210250/ applied, there remain no calls of localtime or gmtime which need to be replaced. Stefan W.
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));
Also changed the caller of gmtime() to gmtime_r(). Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> --- oslib-win32.c | 6 +++++- vl.c | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-)