Re: [Qemu-devel] [PATCH v2 03/21] hw: timer: Add ASPEED RTC device

[Peter Maydell]

On Tue, 18 Jun 2019 at 17:53, Cédric Le Goater <address@hidden> wrote:
>
> From: Joel Stanley <address@hidden>
>
> The RTC is modeled to provide time and date functionality. It is
> initialised at zero to match the hardware.
>
> There is no modelling of the alarm functionality, which includes the IRQ
> line. As there is no guest code to exercise this function that is
> acceptable for now.
>
> Signed-off-by: Joel Stanley <address@hidden>
> Reviewed-by: Peter Maydell <address@hidden>

Hi; Coverity complains about this function (CID 1402782):

> +static void aspeed_rtc_calc_offset(AspeedRtcState *rtc)
> +{
> +    struct tm tm;
> +    uint32_t year, cent;
> +    uint32_t reg1 = rtc->reg[COUNTER1];
> +    uint32_t reg2 = rtc->reg[COUNTER2];
> +
> +    tm.tm_mday = (reg1 >> 24) & 0x1f;
> +    tm.tm_hour = (reg1 >> 16) & 0x1f;
> +    tm.tm_min = (reg1 >> 8) & 0x3f;
> +    tm.tm_sec = (reg1 >> 0) & 0x3f;
> +
> +    cent = (reg2 >> 16) & 0x1f;
> +    year = (reg2 >> 8) & 0x7f;
> +    tm.tm_mon = ((reg2 >>  0) & 0x0f) - 1;
> +    tm.tm_year = year + (cent * 100) - 1900;
> +
> +    rtc->offset = qemu_timedate_diff(&tm);
> +}

because the tm_wday field of 'struct tm tm' is not initialized
before we call qemu_timedate_diff(). This is a false
positive because the "read" of this field is just the place
in qemu_timedate_diff() that does "struct tm tmp = *tm;"
before calling mktime(), and mktime() ignores tm_wday.
We could make Coverity happy by using a struct initializer
on 'tm' here; on the other hand we don't do that anywhere else
which calls qemu_timedate_diff(), so maybe I should just mark
this a false positive?

thanks
-- PMM