[Qemu-devel] [PATCH 1/2] hw/display/cg3.c: cg3_realizefn: fix memory leak, make FCode loading error fatal and improve error message

[Qemu-devel] [PATCH 1/2] hw/display/cg3.c: cg3_realizefn: fix memory leak, make FCode loading error fatal and improve error message

[Michael Tokarev]

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 hw/display/cg3.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index 1e6ff2b..5f869a7 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -302,10 +302,12 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
     if (fcode_filename) {
         ret = load_image_targphys(fcode_filename, s->prom_addr,
                                   FCODE_MAX_ROM_SIZE);
-        if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
-            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
-        }
     }
+    if (!fcode_filename || ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
+        error_report("cg3: could not load prom '%s'",
+                     fcode_filename ? fcode_filename : CG3_ROM_FILE);
+    }
+    g_free(fcode_filename);
 
     memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
                            &error_abort);
-- 
2.1.4

Re: [Qemu-devel] [PATCH 1/2] hw/display/cg3.c: cg3_realizefn: fix memory leak, make FCode loading error fatal and improve error message

[Michael Tokarev]

Shannon Zhao: can you please add your s-o-b or r-o-b for this
version, for this and the next patch?

This started from your original patch which just adds g_free(),
later you also added error message improvements to this piece,
on my request, but apparently this place needs more tweeking,
since failing to load firmware is fatal to the code, so instead
of requesting you to change it further I created my version,
identical for two files.

Thanks,

/mjt

30.05.2015 16:49, Michael Tokarev wrote:
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  hw/display/cg3.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
> index 1e6ff2b..5f869a7 100644
> --- a/hw/display/cg3.c
> +++ b/hw/display/cg3.c
> @@ -302,10 +302,12 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
>      if (fcode_filename) {
>          ret = load_image_targphys(fcode_filename, s->prom_addr,
>                                    FCODE_MAX_ROM_SIZE);
> -        if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
> -            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
> -        }
>      }
> +    if (!fcode_filename || ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
> +        error_report("cg3: could not load prom '%s'",
> +                     fcode_filename ? fcode_filename : CG3_ROM_FILE);
> +    }
> +    g_free(fcode_filename);
>  
>      memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
>                             &error_abort);
> 

Re: [Qemu-devel] [PATCH 1/2] hw/display/cg3.c: cg3_realizefn: fix memory leak, make FCode loading error fatal and improve error message

[Shannon Zhao]

On 2015/5/30 21:49, Michael Tokarev wrote:
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

Reviewed-by: Shannon Zhao <shannon.zhao@linaro.org>

> ---
>  hw/display/cg3.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
> index 1e6ff2b..5f869a7 100644
> --- a/hw/display/cg3.c
> +++ b/hw/display/cg3.c
> @@ -302,10 +302,12 @@ static void cg3_realizefn(DeviceState *dev, Error **errp)
>      if (fcode_filename) {
>          ret = load_image_targphys(fcode_filename, s->prom_addr,
>                                    FCODE_MAX_ROM_SIZE);
> -        if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
> -            error_report("cg3: could not load prom '%s'", CG3_ROM_FILE);
> -        }
>      }
> +    if (!fcode_filename || ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
> +        error_report("cg3: could not load prom '%s'",
> +                     fcode_filename ? fcode_filename : CG3_ROM_FILE);
> +    }
> +    g_free(fcode_filename);
>  
>      memory_region_init_ram(&s->vram_mem, NULL, "cg3.vram", s->vram_size,
>                             &error_abort);
> 

-- 
Shannon