Re: [Qemu-devel] [PATCH v2 1/2] hw/arm: check fw_cfg return value before using it

[Hongbo Zhang]

On 31 July 2018 at 02:07, Peter Maydell <address@hidden> wrote:
> On 25 July 2018 at 06:30, Hongbo Zhang <address@hidden> wrote:
>> The fw_cfg value returned from fw_cfg_find() may be NULL, so check it
>> before using.
>>
>> Signed-off-by: Hongbo Zhang <address@hidden>
>> ---
>>  hw/arm/boot.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index e09201c..43b217f 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -930,6 +930,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
>> *info)
>>      hwaddr entry;
>>      static const ARMInsnFixup *primary_loader;
>>      AddressSpace *as = arm_boot_address_space(cpu, info);
>> +    FWCfgState *fw_cfg;
>>
>>      /* CPU objects (unlike devices) are not automatically reset on system
>>       * reset, so we must always register a handler to do so. If we're
>> @@ -960,11 +961,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
>> *info)
>>              info->dtb_start = info->loader_start;
>>          }
>>
>> -        if (info->kernel_filename) {
>> -            FWCfgState *fw_cfg;
>> +        fw_cfg = fw_cfg_find();
>> +        if (info->kernel_filename && fw_cfg) {
>>              bool try_decompressing_kernel;
>
> This can only happen if:
>  * the user provided a firmware blob
>  * the user provided a -kernel option
>  * the board does not have a fw_cfg so we can't pass the kernel to
>    the firmware blob
> right?
>
Yes.

> I think in this situation we should exit with a helpful error
> message to the user (telling them that this board model does
> not support using the -kernel option when a guest firmware image
> is being booted), rather than silently ignoring the option.
>
The suggestion sounds better.
Thanks.

> thanks
> -- PMM