Patchwork [1/3] use int64 when compare two time

login
register
mail settings
Submitter Zhang, Yang Z
Date Jan. 6, 2012, 7:37 a.m.
Message ID <A9667DDFB95DB7438FA9D7D576C3D87E018CCB@SHSMSX101.ccr.corp.intel.com>
Download mbox | patch
Permalink /patch/134614/
State New
Headers show

Comments

Zhang, Yang Z - Jan. 6, 2012, 7:37 a.m.
use int64 when compare two time

int32 only represent only 136 years when comparing two times based on second. It would be better to use int64.

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Peter Maydell - Jan. 6, 2012, 1:20 p.m.
On 6 January 2012 07:37, Zhang, Yang Z <yang.z.zhang@intel.com> wrote:
> use int64 when compare two time
>
> int32 only represent only 136 years when comparing two times based on second. It would be better to use int64.

"int32", "int32_t" and "'int' which happens to be 32 bit" are all
different types;
your changelog message is confusing them.

Anyway, maybe we should be using time_t here? The functions use that
internally anyway so is there a reason not to just use it in the API too?

-- PMM
Andreas Färber - Jan. 6, 2012, 5:44 p.m.
Am 06.01.2012 08:37, schrieb Zhang, Yang Z:
> use int64 when compare two time
> 
> int32 only represent only 136 years when comparing two times based on second. It would be better to use int64.

int32 and int64 are softfloat types and should not be used here.

Do you have an actual use case that breaks with int / int32_t?

> 
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>

> diff --git a/vl.c b/vl.c
> index 640e3ca..01c5a9d 100644
> --- a/vl.c
> +++ b/vl.c

> @@ -454,7 +454,7 @@ void qemu_get_timedate(struct tm *tm, int offset)
>      memcpy(tm, ret, sizeof(struct tm));
>  }
> 
> -int qemu_timedate_diff(struct tm *tm)
> +int64_t qemu_timedate_diff(struct tm *tm)
>  {
>      time_t seconds;
> 
> @@ -476,7 +476,7 @@ void rtc_change_mon_event(struct tm *tm)
>  {
>      QObject *data;
> 
> -    data = qobject_from_jsonf("{ 'offset': %d }", qemu_timedate_diff(tm));
> +    data = qobject_from_jsonf("{ 'offset': %ld }", qemu_timedate_diff(tm));

That's wrong, %ld is for long. For int64_t you need to use PRId64.

Andreas

>      monitor_protocol_event(QEVENT_RTC_CHANGE, data);
>      qobject_decref(data);
>  }
Zhang, Yang Z - Jan. 10, 2012, 5:18 a.m.
> -----Original Message-----
> From: Andreas Färber [mailto:afaerber@suse.de]
> Sent: Saturday, January 07, 2012 1:44 AM
> > use int64 when compare two time
> >
> > int32 only represent only 136 years when comparing two times based on
> second. It would be better to use int64.
> 
> int32 and int64 are softfloat types and should not be used here.
> 
> Do you have an actual use case that breaks with int / int32_t?
> 
The range of int is from (-2^31~2^31-1). This means it only can express (-2^31~2^31-1) seconds, about -68 years ~ +68 years compare with now. 
You can use "-rtc base=YYYY-MM-DD" to set the RTC start date and if (base - now) greater than 68 year, then you will see error message in windows guest and the date will roll back to 1944.

best regards
yang
Marcelo Tosatti - Jan. 10, 2012, 4:51 p.m.
On Fri, Jan 06, 2012 at 01:20:55PM +0000, Peter Maydell wrote:
> On 6 January 2012 07:37, Zhang, Yang Z <yang.z.zhang@intel.com> wrote:
> > use int64 when compare two time
> >
> > int32 only represent only 136 years when comparing two times based on second. It would be better to use int64.
> 
> "int32", "int32_t" and "'int' which happens to be 32 bit" are all
> different types;
> your changelog message is confusing them.
> 
> Anyway, maybe we should be using time_t here? The functions use that
> internally anyway so is there a reason not to just use it in the API too?

time_t contains seconds since Epoch, which is not the case with
offsets in qemu_get_timedate/qemu_timedate_diff.

Patch

diff --git a/qemu-common.h b/qemu-common.h
index b2de015..c14f506 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -116,8 +116,8 @@  extern int use_icount;
 int qemu_main(int argc, char **argv, char **envp);
 #endif

-void qemu_get_timedate(struct tm *tm, int offset);
-int qemu_timedate_diff(struct tm *tm);
+void qemu_get_timedate(struct tm *tm, int64_t offset);
+int64_t qemu_timedate_diff(struct tm *tm);

 /* cutils.c */
 void pstrcpy(char *buf, int buf_size, const char *str);
diff --git a/vl.c b/vl.c
index 640e3ca..01c5a9d 100644
--- a/vl.c
+++ b/vl.c
@@ -189,7 +189,7 @@  int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int autostart;
 static int rtc_utc = 1;
-static int rtc_date_offset = -1; /* -1 means no change */
+static int64_t rtc_date_offset = -1; /* -1 means no change */
 QEMUClock *rtc_clock;
 int vga_interface_type = VGA_NONE;
 static int full_screen = 0;
@@ -434,7 +434,7 @@  StatusInfo *qmp_query_status(Error **errp)

 /***********************************************************/
 /* host time/date access */
-void qemu_get_timedate(struct tm *tm, int offset)
+void qemu_get_timedate(struct tm *tm, int64_t offset)
 {
     time_t ti;
     struct tm *ret;
@@ -454,7 +454,7 @@  void qemu_get_timedate(struct tm *tm, int offset)
     memcpy(tm, ret, sizeof(struct tm));
 }

-int qemu_timedate_diff(struct tm *tm)
+int64_t qemu_timedate_diff(struct tm *tm)
 {
     time_t seconds;

@@ -476,7 +476,7 @@  void rtc_change_mon_event(struct tm *tm)
 {
     QObject *data;

-    data = qobject_from_jsonf("{ 'offset': %d }", qemu_timedate_diff(tm));
+    data = qobject_from_jsonf("{ 'offset': %ld }", qemu_timedate_diff(tm));
     monitor_protocol_event(QEVENT_RTC_CHANGE, data);
     qobject_decref(data);
 }