diff mbox

[4/4] hw/display/tcx.c: Fix memory leak spotted by valgrind

Message ID 1432811625-13392-5-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao May 28, 2015, 11:13 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>

valgrind complains about:
==23693== 55 bytes in 1 blocks are definitely lost in loss record 1,277 of 2,014
==23693==    at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==23693==    by 0x21B93F: malloc_and_trace (vl.c:2556)
==23693==    by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3)
==23693==    by 0x64DEFD7: g_strndup (in /usr/lib64/libglib-2.0.so.0.3600.3)
==23693==    by 0x650181A: g_vasprintf (in /usr/lib64/libglib-2.0.so.0.3600.3)
==23693==    by 0x64DF0CC: g_strdup_vprintf (in /usr/lib64/libglib-2.0.so.0.3600.3)
==23693==    by 0x64DF188: g_strdup_printf (in /usr/lib64/libglib-2.0.so.0.3600.3)
==23693==    by 0x21A7B1: qemu_find_file (vl.c:2121)
==23693==    by 0x1E4F6E: tcx_realizefn (tcx.c:1013)
==23693==    by 0x26CC20: device_set_realized (qdev.c:1058)
==23693==    by 0x2B6766: property_set_bool (object.c:1514)
==23693==    by 0x2B5060: object_property_set (object.c:837)

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/display/tcx.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Michael Tokarev May 28, 2015, 11:54 a.m. UTC | #1 
Applied to -trivial, thanks!

/mjt
Michael Tokarev May 28, 2015, 12:01 p.m. UTC | #2 
28.05.2015 14:54, Michael Tokarev wrote:
> Applied to -trivial, thanks!

..or not applied yet... :)

This is again the same situation with the error message as with
1/4 hw/display/cg3.c, and the same question - is the error fatal?

And note that you're the number-one person who made changes to
this file... ;)

/mjt
Mark Cave-Ayland May 28, 2015, 12:19 p.m. UTC | #3 
On 28/05/15 13:01, Michael Tokarev wrote:

> 28.05.2015 14:54, Michael Tokarev wrote:
>> Applied to -trivial, thanks!
> 
> ..or not applied yet... :)
> 
> This is again the same situation with the error message as with
> 1/4 hw/display/cg3.c, and the same question - is the error fatal?
> 
> And note that you're the number-one person who made changes to
> this file... ;)
> 
> /mjt

The FCode ROMs supplied for CG3/TCX are used to both initialise various
DT entries for the graphic adapters and also provide a minimal driver to
allow OpenBIOS (or Sun PROM) to initialise and use the adapter at boot.

I'd say at the moment the error should be fatal, since the correct way
to have a headless system would be not to have the virtual graphics
adapter plugged into the system in the first place. Otherwise I can see
that the skeleton DT nodes generated by the PROM would be missing
several properties generated by the FCode (such as the framebuffer
address), which would be utterly confusing for any client trying to use
the graphics adapter.


ATB,

Mark.
Michael Tokarev May 29, 2015, 6:18 a.m. UTC | #4 
28.05.2015 15:19, Mark Cave-Ayland wrote:
[]
> The FCode ROMs supplied for CG3/TCX are used to both initialise various
> DT entries for the graphic adapters and also provide a minimal driver to
> allow OpenBIOS (or Sun PROM) to initialise and use the adapter at boot.
> 
> I'd say at the moment the error should be fatal, [...]

So it looks like some exit(1) or similar is missing after error_report()
in these cases, which should be added :)  Thanks!

/mjt
diff mbox

Patch

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index a9f9f66..6c5e584 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -1014,6 +1014,7 @@  static void tcx_realizefn(DeviceState *dev, Error **errp)
     if (fcode_filename) {
         ret = load_image_targphys(fcode_filename, s->prom_addr,
                                   FCODE_MAX_ROM_SIZE);
+        g_free(fcode_filename);
         if (ret < 0 || ret > FCODE_MAX_ROM_SIZE) {
             error_report("tcx: could not load prom '%s'", TCX_ROM_FILE);
         }