RE: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero

[Jamin Lin]

Hi cmd, Cedric and Peter,

> -----Original Message-----
> From: cmd <clement.mathieudrif.etu@gmail.com>
> Sent: Tuesday, June 25, 2024 2:07 PM
> To: Cédric Le Goater <clg@kaod.org>; Jamin Lin <jamin_lin@aspeedtech.com>;
> Peter Maydell <peter.maydell@linaro.org>; Steven Lee
> <steven_lee@aspeedtech.com>; Troy Lee <leetroy@gmail.com>; Andrew
> Jeffery <andrew@codeconstruct.com.au>; Joel Stanley <joel@jms.id.au>; open
> list:ASPEED BMCs <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Cédric Le Goater <clg@redhat.com>
> Subject: Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
> 
> 
> On 25/06/2024 08:03, Cédric Le Goater wrote:
> > On 6/25/24 8:00 AM, cmd wrote:
> >> Hi
> >>
> >> On 25/06/2024 03:50, Jamin Lin via wrote:
> >>> Coverity reports a possible DIVIDE_BY_ZERO issue regarding the
> >>> "ram_size" object property. This can not happen because RAM has
> >>> predefined valid sizes per SoC. Nevertheless, add a test to close
> >>> the issue.
> >>>
> >>> Fixes: Coverity CID 1547113
> >>> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> >>> Reviewed-by: Cédric Le Goater <clg@redhat.com> [ clg: Rewrote commit
> >>> log ]
> >>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> >>> ---
> >>>   hw/arm/aspeed_ast27x0.c | 6 ++++++
> >>>   1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c index
> >>> b6876b4862..d14a46df6f 100644
> >>> --- a/hw/arm/aspeed_ast27x0.c
> >>> +++ b/hw/arm/aspeed_ast27x0.c
> >>> @@ -211,6 +211,12 @@ static void aspeed_ram_capacity_write(void
> >>> *opaque, hwaddr addr, uint64_t data,
> >>>       ram_size = object_property_get_uint(OBJECT(&s->sdmc),
> >>> "ram-size",
> >>>                                           &error_a
> bort);
> >>> +    if (!ram_size) {
> >>> +        qemu_log_mask(LOG_GUEST_ERROR,
> >>> +                      "%s: ram_size is zero",  __func__);
> >>> +        return;
> >>> +    }
> >>> +
> >> If we are sure that the error cannot happen, shouldn't we assert
> >> instead?
> >
> > Yes. That is what Peter suggested. This needs to be changed.
> >
Thanks for review and suggestion.
How about this change?

assert(ram_size > 0);

If you agree, I will send v3 patch.
Thanks-Jamin

> >
> > Thanks,
> >
> > C.
> >
> Ok fine, I didn't see the message, sorry!
> 
> Thanks
> 
>  >cmd
> 
> >
> >
> >>>       /*
> >>>        * Emulate ddr capacity hardware behavior.
> >>>        * If writes the data to the address which is beyond the ram
> >>> size,
> >