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

[Joel Stanley]

On Tue, 2 Jul 2019 at 19:19, Peter Maydell <address@hidden> wrote:
>
> 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?

I don't have an opinion on which option to take. Perhaps mark it as a
false positive?

Cheers,

Joel