On 1/16/21 11:38 PM, Alistair Francis wrote: > On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé <f4...@amsat.org> > wrote: >> >> On 1/16/21 12:00 AM, Alistair Francis wrote: >>> We were accidently passing RISCVHartArrayState by value instead of >>> pointer. The type is 824 bytes long so let's correct that and pass it by >>> pointer instead. >>> >>> Fixes: Coverity CID 1438099 >>> Fixes: Coverity CID 1438100 >>> Fixes: Coverity CID 1438101 >>> Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> >>> --- >>> include/hw/riscv/boot.h | 6 +++--- >>> hw/riscv/boot.c | 8 ++++---- >>> hw/riscv/sifive_u.c | 10 +++++----- >>> hw/riscv/spike.c | 8 ++++---- >>> hw/riscv/virt.c | 8 ++++---- >>> 5 files changed, 20 insertions(+), 20 deletions(-) ...
>>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >>> index 83586aef41..acf77675b2 100644 >>> --- a/hw/riscv/boot.c >>> +++ b/hw/riscv/boot.c >>> @@ -33,14 +33,14 @@ >>> >>> #include <libfdt.h> >>> >>> -bool riscv_is_32bit(RISCVHartArrayState harts) >>> +bool riscv_is_32bit(RISCVHartArrayState *harts) >>> { >>> - RISCVCPU hart = harts.harts[0]; >>> + RISCVCPU hart = harts->harts[0]; >> >> This doesn't look improved. Maybe you want: >> >> return riscv_cpu_is_32bit(&harts->harts[0].env); > > I suspect this ends up generating the same code. If the compiler is smart enough, but I'm not sure it can figure out only 1 element from the structure is accessed... My understanding is "first copy the content pointed at '*harts' in 'hart' on the stack", then only use "env". Cc'ing Eric/Richard to double check. > > Either way, good point I have just squashed this change into the patch. Thanks, Phil.