[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 1/1] riscv: Pass RISCVHartArrayState by pointer
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v1 1/1] riscv: Pass RISCVHartArrayState by pointer |
Date: |
Sun, 17 Jan 2021 17:52:44 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/16/21 11:38 PM, Alistair Francis wrote:
> On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé <f4bug@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.francis@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.