Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence

[Cédric Le Goater]

On 09/25/2017 10:52 AM, Greg Kurz wrote:
> On Mon, 25 Sep 2017 10:46:13 +0200
> Thomas Huth  wrote:
> 
>> On 25.09.2017 10:33, Cédric Le Goater wrote:
>>> and exit before uslessly trying to load it if the file does  
>>
>> s/uslessly/uselessly/
>>
>>> not exists.
>>>
>>> Issue dicovered by Coverity Scan.  
>>
>> /dicovered/discovered/
>>
> 
> Mentioning the Coverity Id (ie, CID 1381410) is also a usual practice.


ah. too late :/

C. 

>>> Signed-off-by: Cédric Le Goater 
>>> ---
>>>  hw/ppc/pnv.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>> index 47221158d415..7728eece2eec 100644
>>> --- a/hw/ppc/pnv.c
>>> +++ b/hw/ppc/pnv.c
>>> @@ -570,6 +570,10 @@ static void ppc_powernv_init(MachineState *machine)
>>>  }
>>>  
>>>  fw_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>> +if (!fw_filename) {
>>> +error_report("Could not find OPAL '%s'", bios_name);  
>>
>> I'd suggest to say "OPAL firmware" instead of just "OPAL". Not every
>> user knows what OPAL is.
>>
> 
> With all the suggested changes:
> 
> Reviewed-by: Greg Kurz 
> 
>>> +exit(1);
>>> +}
>>>  
>>>  fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
>>>  if (fw_size < 0) {
>>>   
>>
>>  Thomas
> 
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence

[Cédric Le Goater]

On 09/25/2017 10:46 AM, Thomas Huth wrote:
> On 25.09.2017 10:33, Cédric Le Goater wrote:
>> and exit before uslessly trying to load it if the file does
> 
> s/uslessly/uselessly/
> 
>> not exists.
>>
>> Issue dicovered by Coverity Scan.
> 
> /dicovered/discovered/

arg. Sorry about that.

>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/ppc/pnv.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 47221158d415..7728eece2eec 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -570,6 +570,10 @@ static void ppc_powernv_init(MachineState *machine)
>>  }
>>  
>>  fw_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> +if (!fw_filename) {
>> +error_report("Could not find OPAL '%s'", bios_name);
> 
> I'd suggest to say "OPAL firmware" instead of just "OPAL". Not every
> user knows what OPAL is.

Yes it deserves a resend anyhow.

thanks,

C. 

>> +exit(1);
>> +}
>>  
>>  fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
>>  if (fw_size < 0) {
>>
> 
>  Thomas
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence

[Greg Kurz]

On Mon, 25 Sep 2017 10:46:13 +0200
Thomas Huth  wrote:

> On 25.09.2017 10:33, Cédric Le Goater wrote:
> > and exit before uslessly trying to load it if the file does  
> 
> s/uslessly/uselessly/
> 
> > not exists.
> > 
> > Issue dicovered by Coverity Scan.  
> 
> /dicovered/discovered/
> 

Mentioning the Coverity Id (ie, CID 1381410) is also a usual practice.

> > Signed-off-by: Cédric Le Goater 
> > ---
> >  hw/ppc/pnv.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 47221158d415..7728eece2eec 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -570,6 +570,10 @@ static void ppc_powernv_init(MachineState *machine)
> >  }
> >  
> >  fw_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> > +if (!fw_filename) {
> > +error_report("Could not find OPAL '%s'", bios_name);  
> 
> I'd suggest to say "OPAL firmware" instead of just "OPAL". Not every
> user knows what OPAL is.
> 

With all the suggested changes:

Reviewed-by: Greg Kurz 

> > +exit(1);
> > +}
> >  
> >  fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
> >  if (fw_size < 0) {
> >   
> 
>  Thomas



-- 
Gregory Kurz kurzg...@fr.ibm.com
 gk...@linux.vnet.ibm.com
Software Engineer @ IBM/LTC  http://www.ibm.com
Tel 33-5-6218-1607

"Anarchy is about taking complete responsibility for yourself."
Alan Moore.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence

[Thomas Huth]

On 25.09.2017 10:33, Cédric Le Goater wrote:
> and exit before uslessly trying to load it if the file does

s/uslessly/uselessly/

> not exists.
> 
> Issue dicovered by Coverity Scan.

/dicovered/discovered/

> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ppc/pnv.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 47221158d415..7728eece2eec 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -570,6 +570,10 @@ static void ppc_powernv_init(MachineState *machine)
>  }
>  
>  fw_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +if (!fw_filename) {
> +error_report("Could not find OPAL '%s'", bios_name);

I'd suggest to say "OPAL firmware" instead of just "OPAL". Not every
user knows what OPAL is.

> +exit(1);
> +}
>  
>  fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
>  if (fw_size < 0) {
> 

 Thomas