diff mbox series

[for-8.2,1/4] hw/rtc/m48t59: Use 64-bit arithmetic in set_alarm()

Message ID 20230720155902.1590362-2-peter.maydell@linaro.org
State Superseded
Headers show
Series rtc devices: Avoid putting time_t in 32-bit variables | expand

Commit Message

Peter Maydell July 20, 2023, 3:58 p.m. UTC 
In the m48t59 device we almost always use 64-bit arithmetic when
dealing with time_t deltas.  The one exception is in set_alarm(),
which currently uses a plain 'int' to hold the difference between two
time_t values.  Switch to int64_t instead to avoid any possible
overflow issues.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/rtc/m48t59.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé July 21, 2023, 9:03 a.m. UTC | #1 
On 20/7/23 17:58, Peter Maydell wrote:
> In the m48t59 device we almost always use 64-bit arithmetic when
> dealing with time_t deltas.  The one exception is in set_alarm(),
> which currently uses a plain 'int' to hold the difference between two
> time_t values.  Switch to int64_t instead to avoid any possible
> overflow issues.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/rtc/m48t59.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Philippe Mathieu-Daudé July 21, 2023, 9:09 a.m. UTC | #2 
On 20/7/23 17:58, Peter Maydell wrote:
> In the m48t59 device we almost always use 64-bit arithmetic when
> dealing with time_t deltas.  The one exception is in set_alarm(),
> which currently uses a plain 'int' to hold the difference between two
> time_t values.  Switch to int64_t instead to avoid any possible
> overflow issues.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/rtc/m48t59.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Similarly:

-- >8 --
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -88,7 +88,7 @@ static M48txxInfo m48txx_sysbus_info[] = {
  static void alarm_cb (void *opaque)
  {
      struct tm tm;
-    uint64_t next_time;
+    int64_t next_time;
      M48t59State *NVRAM = opaque;
---
Peter Maydell July 21, 2023, 9:42 a.m. UTC | #3 
On Fri, 21 Jul 2023 at 10:09, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 20/7/23 17:58, Peter Maydell wrote:
> > In the m48t59 device we almost always use 64-bit arithmetic when
> > dealing with time_t deltas.  The one exception is in set_alarm(),
> > which currently uses a plain 'int' to hold the difference between two
> > time_t values.  Switch to int64_t instead to avoid any possible
> > overflow issues.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   hw/rtc/m48t59.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
>
> Similarly:
>
> -- >8 --
> --- a/hw/rtc/m48t59.c
> +++ b/hw/rtc/m48t59.c
> @@ -88,7 +88,7 @@ static M48txxInfo m48txx_sysbus_info[] = {
>   static void alarm_cb (void *opaque)
>   {
>       struct tm tm;
> -    uint64_t next_time;
> +    int64_t next_time;
>       M48t59State *NVRAM = opaque;

Why? next_time should always be positive here, I think.

-- PMM
diff mbox series

Patch

diff --git a/hw/rtc/m48t59.c b/hw/rtc/m48t59.c
index ec3e56e84fd..2e2c849985c 100644
--- a/hw/rtc/m48t59.c
+++ b/hw/rtc/m48t59.c
@@ -133,7 +133,7 @@  static void alarm_cb (void *opaque)
 
 static void set_alarm(M48t59State *NVRAM)
 {
-    int diff;
+    int64_t diff;
     if (NVRAM->alrm_timer != NULL) {
         timer_del(NVRAM->alrm_timer);
         diff = qemu_timedate_diff(&NVRAM->alarm) - NVRAM->time_offset;