[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-8.2 0/4] rtc devices: Avoid putting time_t in 32-bit vari
From: |
Peter Maydell |
Subject: |
Re: [PATCH for-8.2 0/4] rtc devices: Avoid putting time_t in 32-bit variables |
Date: |
Fri, 21 Jul 2023 10:45:07 +0100 |
On Fri, 21 Jul 2023 at 10:03, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> +Markus
>
> On 20/7/23 17:58, Peter Maydell wrote:
> > This patchset was prompted by a couple of Coverity warnings
> > (CID 1507157, 1517772) which note that in the m48t59 RTC device model
> > we keep an offset in a time_t variable but then truncate it by
> > passing it to qemu_get_timedate(), which currently uses an 'int'
> > argument for its offset parameter.
> >
> > We can fix the Coverity complaint by making qemu_get_timedate()
> > take a time_t; we should also correspondingly make the
> > qemu_timedate_diff() function return a time_t. However this
> > will only push the issue out to callers of qemu_timedate_diff()
> > if they are putting the result in a 32-bit variable or doing
> > 32-bit arithmetic on it.
> >
> > Luckily there aren't that many callers of qemu_timedate_diff()
> > and most of them already use either time_t or int64_t for the
> > calculations they do on its return value. The first three
> > patches fix devices which weren't doing that; patch four then
> > fixes the rtc.c functions. If I missed any callsites in devices
> > then hopefully Coverity will point them out.
>
> Do we need to change the type of the RTC_CHANGE event?
>
> This is wrong, but to give an idea:
>
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -553,47 +553,47 @@
> ##
> # @RTC_CHANGE:
> #
> # Emitted when the guest changes the RTC time.
> #
> # @offset: offset in seconds between base RTC clock (as specified by
> # -rtc base), and new RTC clock value
> #
> # @qom-path: path to the RTC object in the QOM tree
> #
> # Note: This event is rate-limited. It is not guaranteed that the RTC
> # in the system implements this event, or even that the system has
> # an RTC at all.
> #
> # Since: 0.13
> #
> # Example:
> #
> # <- { "event": "RTC_CHANGE",
> # "data": { "offset": 78 },
> # "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
> ##
> { 'event': 'RTC_CHANGE',
> - 'data': { 'offset': 'int', 'qom-path': 'str' } }
> + 'data': { 'offset': 'int64', 'qom-path': 'str' } }
> ---
If I understand the 'Built-in Types' section in
docs/devel/qapi-code-gen.rst correctly, the QAPI 'int'
type is already 64 bits.
thanks
-- PMM