| Message ID | 4cba4d80e570372183b6685a26c72fa3e907bb51.1570051975.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/1] riscv/boot: Fix possible memory leak | expand |
On 10/2/19 2:34 PM, Alistair Francis wrote: > Coverity (CID 1405786) thinks that there is a possible memory leak as > we don't guarentee that the memory allocatd from riscv_find_firmware() > is freed. This is a false positive, but let's tidy up the code to fix > the warning. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Thu, Oct 3, 2019 at 5:38 AM Alistair Francis <alistair.francis@wdc.com> wrote: > > Coverity (CID 1405786) thinks that there is a possible memory leak as > we don't guarentee that the memory allocatd from riscv_find_firmware() > is freed. This is a false positive, but let's tidy up the code to fix > the warning. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Thanks for the patch. I am not sure how I can easily run Coverity to verify the fix though. Regards, Bin
On 10/2/19 11:34 PM, Alistair Francis wrote: > Coverity (CID 1405786) thinks that there is a possible memory leak as > we don't guarentee that the memory allocatd from riscv_find_firmware() typos: 'guarantee', 'allocated' > is freed. This is a false positive, but let's tidy up the code to fix > the warning. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 2e92fb0680..7fee98d2f8 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -38,7 +38,7 @@ void riscv_find_and_load_firmware(MachineState *machine, > const char *default_machine_firmware, > hwaddr firmware_load_addr) > { > - char *firmware_filename; > + char *firmware_filename = NULL; > > if (!machine->firmware) { > /* > @@ -70,14 +70,11 @@ void riscv_find_and_load_firmware(MachineState *machine, > * if no -bios option is set without breaking anything. > */ > firmware_filename = riscv_find_firmware(default_machine_firmware); > - } else { > - firmware_filename = machine->firmware; > - if (strcmp(firmware_filename, "none")) { > - firmware_filename = riscv_find_firmware(firmware_filename); > - } > + } else if (strcmp(machine->firmware, "none")) { > + firmware_filename = riscv_find_firmware(machine->firmware); > } > > - if (strcmp(firmware_filename, "none")) { > + if (firmware_filename) { > /* If not "none" load the firmware */ > riscv_load_firmware(firmware_filename, firmware_load_addr); > g_free(firmware_filename); > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Thu, 3 Oct 2019 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote: > > On Thu, Oct 3, 2019 at 5:38 AM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > Coverity (CID 1405786) thinks that there is a possible memory leak as > > we don't guarentee that the memory allocatd from riscv_find_firmware() > > is freed. This is a false positive, but let's tidy up the code to fix > > the warning. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/boot.c | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> > > Thanks for the patch. I am not sure how I can easily run Coverity to > verify the fix though. You can't; we use the free 'coverity scan' service which just checks QEMU git master. So we make the fixes we think will deal with the issues, commit them to master in the usual way, and then when the scan is rerun we can have another go if the coverity issue hasn't gone away. thanks -- PMM
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 2e92fb0680..7fee98d2f8 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -38,7 +38,7 @@ void riscv_find_and_load_firmware(MachineState *machine, const char *default_machine_firmware, hwaddr firmware_load_addr) { - char *firmware_filename; + char *firmware_filename = NULL; if (!machine->firmware) { /* @@ -70,14 +70,11 @@ void riscv_find_and_load_firmware(MachineState *machine, * if no -bios option is set without breaking anything. */ firmware_filename = riscv_find_firmware(default_machine_firmware); - } else { - firmware_filename = machine->firmware; - if (strcmp(firmware_filename, "none")) { - firmware_filename = riscv_find_firmware(firmware_filename); - } + } else if (strcmp(machine->firmware, "none")) { + firmware_filename = riscv_find_firmware(machine->firmware); } - if (strcmp(firmware_filename, "none")) { + if (firmware_filename) { /* If not "none" load the firmware */ riscv_load_firmware(firmware_filename, firmware_load_addr); g_free(firmware_filename);
Coverity (CID 1405786) thinks that there is a possible memory leak as we don't guarentee that the memory allocatd from riscv_find_firmware() is freed. This is a false positive, but let's tidy up the code to fix the warning. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/boot.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)