1. Patchew
  2. QEMU
  3. View series

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

Cédric Le Goater posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170925083348.17126-1-clg@kaod.org
Test checkpatch passed
Test docker passed
Test s390x passed
hw/ppc/pnv.c | 4 ++++
1 file changed, 4 insertions(+)
[Qemu-devel] [PATCH] ppc/pnv: check for firmware file presence
Posted by Cédric Le Goater 6 years, 6 months ago 
and exit before uslessly trying to load it if the file does
not exists.

Issue dicovered by Coverity Scan.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 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);
+        exit(1);
+    }
 
     fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
     if (fw_size < 0) {
-- 
2.13.5
Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence
Posted by Thomas Huth 6 years, 6 months ago 
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 <clg@kaod.org>
> ---
>  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
Re: [Qemu-devel] [Qemu-ppc] [PATCH] ppc/pnv: check for firmware file presence
Posted by Cédric Le Goater 6 years, 6 months ago 
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 <clg@kaod.org>
>> ---
>>  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
Posted by Greg Kurz 6 years, 6 months ago 
On Mon, 25 Sep 2017 10:46:13 +0200
Thomas Huth <thuth@redhat.com> 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 <clg@kaod.org>
> > ---
> >  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 <groug@kaod.org>

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



-- 
Gregory Kurz                                     kurzgreg@fr.ibm.com
                                                 gkurz@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
Posted by Cédric Le Goater 6 years, 6 months ago 
On 09/25/2017 10:52 AM, Greg Kurz wrote:
> On Mon, 25 Sep 2017 10:46:13 +0200
> Thomas Huth <thuth@redhat.com> 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 <clg@kaod.org>
>>> ---
>>>  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 <groug@kaod.org>
> 
>>> +        exit(1);
>>> +    }
>>>  
>>>      fw_size = load_image_targphys(fw_filename, FW_LOAD_ADDR, FW_MAX_SIZE);
>>>      if (fw_size < 0) {
>>>   
>>
>>  Thomas
> 
> 
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.