[PATCH 1/7] qemu-common: Briefly document qemu_timedate_diff() unit

Philippe Mathieu-Daudé posted 7 patches 5 years, 7 months ago
[PATCH 1/7] qemu-common: Briefly document qemu_timedate_diff() unit
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago
It is not obvious that the qemu_timedate_diff() and
qemu_ref_timedate() functions return seconds. Briefly
document it.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/qemu-common.h | 1 +
 softmmu/vl.c          | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index d0142f29ac..e97644710c 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -27,6 +27,7 @@ int qemu_main(int argc, char **argv, char **envp);
 #endif
 
 void qemu_get_timedate(struct tm *tm, int offset);
+/* Returns difference with RTC reference time (in seconds) */
 int qemu_timedate_diff(struct tm *tm);
 
 void *qemu_oom_check(void *ptr);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index f669c06ede..215459c7b5 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -737,7 +737,7 @@ void qemu_system_vmstop_request(RunState state)
 }
 
 /***********************************************************/
-/* RTC reference time/date access */
+/* RTC reference time/date access (in seconds) */
 static time_t qemu_ref_timedate(QEMUClockType clock)
 {
     time_t value = qemu_clock_get_ms(clock) / 1000;
-- 
2.21.3


Re: [PATCH 1/7] qemu-common: Briefly document qemu_timedate_diff() unit
Posted by Markus Armbruster 5 years, 7 months ago
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> It is not obvious that the qemu_timedate_diff() and
> qemu_ref_timedate() functions return seconds. Briefly
> document it.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/qemu-common.h | 1 +
>  softmmu/vl.c          | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index d0142f29ac..e97644710c 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -27,6 +27,7 @@ int qemu_main(int argc, char **argv, char **envp);
>  #endif
>  
>  void qemu_get_timedate(struct tm *tm, int offset);
> +/* Returns difference with RTC reference time (in seconds) */
>  int qemu_timedate_diff(struct tm *tm);

Not this patch's problem: use of int here smells; is it wide enough?

>  
>  void *qemu_oom_check(void *ptr);
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index f669c06ede..215459c7b5 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -737,7 +737,7 @@ void qemu_system_vmstop_request(RunState state)
>  }
>  
>  /***********************************************************/
> -/* RTC reference time/date access */
> +/* RTC reference time/date access (in seconds) */
>  static time_t qemu_ref_timedate(QEMUClockType clock)
>  {
>      time_t value = qemu_clock_get_ms(clock) / 1000;

time_t is seconds on all systems we support.  Using it for something
other than seconds would be wrong.  The comment feels redundant to me.
But if it helps someone else...


Re: [PATCH 1/7] qemu-common: Briefly document qemu_timedate_diff() unit
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago
On 6/18/20 7:47 AM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
>> It is not obvious that the qemu_timedate_diff() and
>> qemu_ref_timedate() functions return seconds. Briefly
>> document it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/qemu-common.h | 1 +
>>  softmmu/vl.c          | 2 +-
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qemu-common.h b/include/qemu-common.h
>> index d0142f29ac..e97644710c 100644
>> --- a/include/qemu-common.h
>> +++ b/include/qemu-common.h
>> @@ -27,6 +27,7 @@ int qemu_main(int argc, char **argv, char **envp);
>>  #endif
>>  
>>  void qemu_get_timedate(struct tm *tm, int offset);
>> +/* Returns difference with RTC reference time (in seconds) */
>>  int qemu_timedate_diff(struct tm *tm);
> 
> Not this patch's problem: use of int here smells; is it wide enough?

I'll add a /* FIXME */ comment.

> 
>>  
>>  void *qemu_oom_check(void *ptr);
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index f669c06ede..215459c7b5 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -737,7 +737,7 @@ void qemu_system_vmstop_request(RunState state)
>>  }
>>  
>>  /***********************************************************/
>> -/* RTC reference time/date access */
>> +/* RTC reference time/date access (in seconds) */
>>  static time_t qemu_ref_timedate(QEMUClockType clock)
>>  {
>>      time_t value = qemu_clock_get_ms(clock) / 1000;
> 
> time_t is seconds on all systems we support.  Using it for something
> other than seconds would be wrong.  The comment feels redundant to me.
> But if it helps someone else...

Ah, TIL 'time_t' is the arithmetic time type to represent
the number of seconds since the epoch.

I guess I almost never used it ... (Not something real time
embedded systems care much) :)

So scratch that comment.

> 
>