[Qemu-devel] [PATCH 0/9] Coverity defects fixes

[Qemu-devel] [PATCH 0/9] Coverity defects fixes

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

All those defects are spotted by Coverity, which
classed in high impact outstanding defects.
Reports come from scan.coverity.com for Qemu.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-trivial@nongnu.org

Gonglei (9):
  nbd: fix resource leak
  arm: fix memory leak
  sparc/leon3.c: fix memory leak
  macio: fix possible memory leak
  e500: fix memory leak
  9pfs: fix memory leak
  milkymist.c: fix memory leak
  sysbus: fix memory leak
  microblaze: fix memory leak

 block/nbd.c               |  1 +
 hw/9pfs/virtio-9p-local.c | 10 ++--------
 hw/arm/digic_boards.c     |  1 +
 hw/arm/highbank.c         |  1 +
 hw/arm/vexpress.c         |  1 +
 hw/arm/virt.c             |  1 +
 hw/core/sysbus.c          |  2 ++
 hw/lm32/milkymist.c       |  1 +
 hw/microblaze/boot.c      |  4 +++-
 hw/misc/macio/macio.c     |  3 ++-
 hw/ppc/e500.c             |  1 +
 hw/sparc/leon3.c          |  1 +
 12 files changed, 17 insertions(+), 10 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/9] nbd: fix resource leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 2f3b9ad..b9955cb 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -273,6 +273,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
      */
     sock = nbd_establish_connection(bs, errp);
     if (sock < 0) {
+        g_free(export);
         return sock;
     }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH 2/9] arm: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/arm/digic_boards.c | 1 +
 hw/arm/highbank.c     | 1 +
 hw/arm/vexpress.c     | 1 +
 hw/arm/virt.c         | 1 +
 4 files changed, 4 insertions(+)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 2a4b872..79c453a 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -113,6 +113,7 @@ static void digic_load_rom(DigicBoardState *s, hwaddr addr,
             error_report("Couldn't load rom image '%s'.\n", filename);
             exit(1);
         }
+        g_free(fn);
     }
 }
 
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index a92cdc3..ddd10dc 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -282,6 +282,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
             if (load_image_targphys("sysram.bin", 0xfff88000, filesize) < 0) {
                 hw_error("Unable to load %s\n", bios_name);
             }
+            g_free(sysboot_filename);
         } else {
            hw_error("Unable to find %s\n", bios_name);
         }
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 5933454..3b194b4 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -576,6 +576,7 @@ static void vexpress_common_init(MachineState *machine)
             error_report("Could not load ROM image '%s'", bios_name);
             exit(1);
         }
+        g_free(fn);
     }
 
     /* Motherboard peripherals: the wiring is the same but the
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 69f51ac..5988423 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -565,6 +565,7 @@ static void create_flash(const VirtBoardInfo *vbi)
             error_report("Could not load ROM image '%s'", bios_name);
             exit(1);
         }
+        g_free(fn);
     }
 
     create_one_flash("virt.flash0", flashbase, flashsize);
-- 
1.7.12.4

[Qemu-devel] [PATCH 3/9] sparc/leon3.c: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/sparc/leon3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index 751392e..e41ec0b 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -186,6 +186,7 @@ static void leon3_generic_hw_init(MachineState *machine)
         fprintf(stderr, "Can't read bios image %s\n", filename);
         exit(1);
     }
+    g_free(filename);
 
     /* Can directly load an application. */
     if (kernel_filename != NULL) {
-- 
1.7.12.4

[Qemu-devel] [PATCH 4/9] macio: fix possible memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

If ret = macio_initfn_ide() is less than 0, the timer_memory
will leak the memory it points to.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/misc/macio/macio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index e0f1e88..bf96844 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -273,7 +273,7 @@ static int macio_newworld_initfn(PCIDevice *d)
     MacIOState *s = MACIO(d);
     NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
     SysBusDevice *sysbus_dev;
-    MemoryRegion *timer_memory = g_new(MemoryRegion, 1);
+    MemoryRegion *timer_memory = NULL;
     int i;
     int cur_irq = 0;
     int ret = macio_common_initfn(d);
@@ -301,6 +301,7 @@ static int macio_newworld_initfn(PCIDevice *d)
     }
 
     /* Timer */
+    timer_memory = g_new(MemoryRegion, 1);
     memory_region_init_io(timer_memory, OBJECT(s), &timer_ops, NULL, "timer",
                           0x1000);
     memory_region_add_subregion(&s->bar, 0x15000, timer_memory);
-- 
1.7.12.4

[Qemu-devel] [PATCH 5/9] e500: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/ppc/e500.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 7e17d18..38203a6 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -542,6 +542,7 @@ done:
 
 out:
     g_free(pci_map);
+    g_free(filename);
 
     return ret;
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH 6/9] 9pfs: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/9pfs/virtio-9p-local.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index a183eee..bcad4e0 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_set_xattr(buffer, credp);
@@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_set_mapped_file_attr(fs_ctx, path, credp);
@@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         buffer = rpath(fs_ctx, path);
         err = mknod(buffer, credp->fc_mode, credp->fc_rdev);
         if (err == -1) {
-            g_free(buffer);
             goto out;
         }
         err = local_post_create_passthrough(fs_ctx, path, credp);
@@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
 err_end:
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
@@ -676,7 +673,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -691,7 +687,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -707,7 +702,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
         buffer = rpath(fs_ctx, path);
         fd = open(buffer, flags, credp->fc_mode);
         if (fd == -1) {
-            g_free(buffer);
             err = fd;
             goto out;
         }
@@ -725,8 +719,8 @@ err_end:
     close(fd);
     remove(buffer);
     errno = serrno;
-    g_free(buffer);
 out:
+    g_free(buffer);
     v9fs_string_free(&fullname);
     return err;
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH 7/9] milkymist.c: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/lm32/milkymist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 256c102..7f62261 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -155,6 +155,7 @@ milkymist_init(MachineState *machine)
                 bios_name);
         exit(1);
     }
+    g_free(bios_filename);
 
     milkymist_uart_create(0x60000000, irq[0]);
     milkymist_sysctl_create(0x60001000, irq[1], irq[2], irq[3],
-- 
1.7.12.4

[Qemu-devel] [PATCH 8/9] sysbus: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/core/sysbus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 84af593..b53c351 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -91,6 +91,8 @@ bool sysbus_has_irq(SysBusDevice *dev, int n)
     ObjectProperty *r;
 
     r = object_property_find(OBJECT(dev), prop, NULL);
+    g_free(prop);
+
     return (r != NULL);
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH 9/9] microblaze: fix memory leak

[arei.gonglei]

From: Gonglei <arei.gonglei@huawei.com>

When not assign a -dtb argument, the variable dtb_filename
storage returned from qemu_find_file(), which should be freed
after using.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 hw/microblaze/boot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index a2843cd..4c1e3a9 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -205,5 +205,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr ddr_base,
                             kernel_cmdline,
                             dtb_filename);
     }
-
+    if (!dtb_arg) {
+        g_free(dtb_filename);
+    }
 }
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 5/9] e500: fix memory leak

[Michael Tokarev]

27.02.2015 10:50, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/ppc/e500.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 7e17d18..38203a6 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -542,6 +542,7 @@ done:
>  
>  out:
>      g_free(pci_map);
> +    g_free(filename);

This patch breaks compilation, because `filename' variable is
local to the "if (dtb_file)" code block at the beginning of this
function and shold be freed there.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 6/9] 9pfs: fix memory leak

[Michael Tokarev]

27.02.2015 10:50, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/9pfs/virtio-9p-local.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index a183eee..bcad4e0 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
>          err = local_set_xattr(buffer, credp);
> @@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
>          err = local_set_mapped_file_attr(fs_ctx, path, credp);
> @@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, credp->fc_mode, credp->fc_rdev);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
...

I wonder if we should change this code to drop repeated code blocks which changes
only slightly by mknod parameters...

>          err = local_post_create_passthrough(fs_ctx, path, credp);
> @@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>  err_end:
>      remove(buffer);
>      errno = serrno;
> -    g_free(buffer);
>  out:
> +    g_free(buffer);
>      v9fs_string_free(&fullname);
>      return err;
>  }

..and this goto jumping (here and in a few other places) is q bit disgusting too... ;)

But this is the original code, the patch is still correct.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 2/9] arm: fix memory leak

[Michael Tokarev]

This patch does not apply to current tree.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 0/9] Coverity defects fixes

[Michael Tokarev]

27.02.2015 10:50, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> All those defects are spotted by Coverity, which
> classed in high impact outstanding defects.
> Reports come from scan.coverity.com for Qemu.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: qemu-trivial@nongnu.org
> 
> Gonglei (9):
>   nbd: fix resource leak
Applied.

>   arm: fix memory leak
Does not apply.

>   sparc/leon3.c: fix memory leak
Applied.

>   macio: fix possible memory leak
Applied.

>   e500: fix memory leak
The patch is wrong.

>   9pfs: fix memory leak
Applied.

>   milkymist.c: fix memory leak
Applied.

>   sysbus: fix memory leak
Applied.

>   microblaze: fix memory leak
Applied.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 5/9] e500: fix memory leak

[Gonglei]

On 2015/2/28 17:41, Michael Tokarev wrote:
> 27.02.2015 10:50, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  hw/ppc/e500.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 7e17d18..38203a6 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -542,6 +542,7 @@ done:
>>  
>>  out:
>>      g_free(pci_map);
>> +    g_free(filename);
> 
> This patch breaks compilation, because `filename' variable is
> local to the "if (dtb_file)" code block at the beginning of this
> function and shold be freed there.
> 
Yes, it is. Thanks for your review, I'll respin it.

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH 2/9] arm: fix memory leak

[Gonglei]

On 2015/2/28 17:52, Michael Tokarev wrote:
> This patch does not apply to current tree.
> 
You meant this one should be accepted by arm tree?

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH 0/9] Coverity defects fixes

[Gonglei]

On 2015/2/28 17:54, Michael Tokarev wrote:
> 27.02.2015 10:50, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> All those defects are spotted by Coverity, which
>> classed in high impact outstanding defects.
>> Reports come from scan.coverity.com for Qemu.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: qemu-trivial@nongnu.org
>>
>> Gonglei (9):
>>   nbd: fix resource leak
> Applied.
> 
>>   arm: fix memory leak
> Does not apply.
> 
>>   sparc/leon3.c: fix memory leak
> Applied.
> 
>>   macio: fix possible memory leak
> Applied.
> 
>>   e500: fix memory leak
> The patch is wrong.
> 
>>   9pfs: fix memory leak
> Applied.
> 
>>   milkymist.c: fix memory leak
> Applied.
> 
>>   sysbus: fix memory leak
> Applied.
> 
>>   microblaze: fix memory leak
> Applied.
> 
> Thanks,
> 
> /mjt
> 
Thanks for your work :)

Regards,
-Gonglei

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Michael Tokarev]

28.02.2015 12:59, Gonglei wrote:
> On 2015/2/28 17:52, Michael Tokarev wrote:
>> This patch does not apply to current tree.
>>
> You meant this one should be accepted by arm tree?

Nope. I mean this patch does not apply to my tree.  I found the
problem -- it is because of "Remove superfluous '\n' around error_report()"
series which changed the context slightly.  I applied this one
too.

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Gonglei]

On 2015/2/28 18:18, Michael Tokarev wrote:
> 28.02.2015 12:59, Gonglei wrote:
>> On 2015/2/28 17:52, Michael Tokarev wrote:
>>> This patch does not apply to current tree.
>>>
>> You meant this one should be accepted by arm tree?
> 
> Nope. I mean this patch does not apply to my tree.  I found the
> problem -- it is because of "Remove superfluous '\n' around error_report()"
> series which changed the context slightly.  I applied this one
> too.
> 
ok, thank you.

Regards,
-Gonglei

Re: [Qemu-devel] [Qemu-trivial] [PATCH 6/9] 9pfs: fix memory leak

[Michael Tokarev]

27.02.2015 10:50, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  hw/9pfs/virtio-9p-local.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index a183eee..bcad4e0 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -500,7 +500,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
>          err = local_set_xattr(buffer, credp);
> @@ -513,7 +512,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, SM_LOCAL_MODE_BITS|S_IFREG, 0);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
>          err = local_set_mapped_file_attr(fs_ctx, path, credp);
> @@ -526,7 +524,6 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>          buffer = rpath(fs_ctx, path);
>          err = mknod(buffer, credp->fc_mode, credp->fc_rdev);
>          if (err == -1) {
> -            g_free(buffer);
>              goto out;
>          }
>          err = local_post_create_passthrough(fs_ctx, path, credp);
> @@ -540,8 +537,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
>  err_end:
>      remove(buffer);
>      errno = serrno;
> -    g_free(buffer);
>  out:
> +    g_free(buffer);
>      v9fs_string_free(&fullname);
>      return err;
>  }
> @@ -676,7 +673,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
>          buffer = rpath(fs_ctx, path);
>          fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
>          if (fd == -1) {
> -            g_free(buffer);
>              err = fd;
>              goto out;
>          }
> @@ -691,7 +687,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
>          buffer = rpath(fs_ctx, path);
>          fd = open(buffer, flags, SM_LOCAL_MODE_BITS);
>          if (fd == -1) {
> -            g_free(buffer);
>              err = fd;
>              goto out;
>          }
> @@ -707,7 +702,6 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
>          buffer = rpath(fs_ctx, path);
>          fd = open(buffer, flags, credp->fc_mode);
>          if (fd == -1) {
> -            g_free(buffer);
>              err = fd;
>              goto out;
>          }
> @@ -725,8 +719,8 @@ err_end:
>      close(fd);
>      remove(buffer);
>      errno = serrno;
> -    g_free(buffer);
>  out:
> +    g_free(buffer);
>      v9fs_string_free(&fullname);
>      return err;
>  }

This patch introduces a compiler warning, which is technically
correct:

hw/9pfs/virtio-9p-local.c: In function ‘local_open2’:
hw/9pfs/virtio-9p-local.c:723:5: error: ‘buffer’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     g_free(buffer);
     ^
hw/9pfs/virtio-9p-local.c: In function ‘local_mknod’:
hw/9pfs/virtio-9p-local.c:541:5: error: ‘buffer’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     g_free(buffer);
     ^

This is because `buffer' variable is initialized within one
of the `if' branches but is left uninitialized if none of
the conditions is true.  In reality this can't happen because
at least one condition is always true.

It is a one more reason to rewrite this code like I mentioned
in another mail instead of trying to fix random issues like
this.  I'll try to come up with a patch doing that shortly.
Meanwhile I'll drop this patch.

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Michael Tokarev]

27.02.2015 10:50, arei.gonglei@huawei.com wrote:
[]
> @@ -565,6 +565,7 @@ static void create_flash(const VirtBoardInfo *vbi)
>              error_report("Could not load ROM image '%s'", bios_name);
>              exit(1);
>          }
> +        g_free(fn);
>      }
>  
>      create_one_flash("virt.flash0", flashbase, flashsize);
> 

This change introduces a compiler error:

hw/arm/virt.c: In function ‘create_flash’:
hw/arm/virt.c:568:16: error: passing argument 1 of ‘g_free’ discards ‘const’ qualifier from pointer target type [-Werror]
         g_free(fn);
                ^
/usr/include/glib-2.0/glib/gmem.h:69:7: note: expected ‘gpointer’ but argument is of type ‘const char *’
 void  g_free           (gpointer  mem);
       ^

This is because just a few lines above, `fn' variable
is declared as `const char *', which obviously should
not be used with any *free() functions.

The solution is to remove `const' from the variable
declaration.

And a much more general solution is to actually test
patches before submitting them.  You obviously did not
test this series, having 3 errors in 9 patches, ie,
1/3 of your patches does not work...

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Gonglei]

On 2015/3/4 21:16, Michael Tokarev wrote:
> 27.02.2015 10:50, arei.gonglei@huawei.com wrote:
> []
>> @@ -565,6 +565,7 @@ static void create_flash(const VirtBoardInfo *vbi)
>>              error_report("Could not load ROM image '%s'", bios_name);
>>              exit(1);
>>          }
>> +        g_free(fn);
>>      }
>>  
>>      create_one_flash("virt.flash0", flashbase, flashsize);
>>
> 
> This change introduces a compiler error:
> 
> hw/arm/virt.c: In function ‘create_flash’:
> hw/arm/virt.c:568:16: error: passing argument 1 of ‘g_free’ discards ‘const’ qualifier from pointer target type [-Werror]
>          g_free(fn);
>                 ^
> /usr/include/glib-2.0/glib/gmem.h:69:7: note: expected ‘gpointer’ but argument is of type ‘const char *’
>  void  g_free           (gpointer  mem);
>        ^
> 
> This is because just a few lines above, `fn' variable
> is declared as `const char *', which obviously should
> not be used with any *free() functions.
> 
Yes, and I found another similar problem in
 [PATCH 9/9] microblaze: fix memory leak
which you had pulled, maybe make sense wait a minuter.
Sorry for this.
> The solution is to remove `const' from the variable
> declaration.
> 
Correct.
> And a much more general solution is to actually test
> patches before submitting them.  You obviously did not
> test this series, having 3 errors in 9 patches, ie,
> 1/3 of your patches does not work...
> 
Apologize earnestly for troubling you. :(

I just thought they are so trivial, meanwhile I hadn't arm compiling environment (lack of
libfdt-dev), and assigned "--target-list=x86_64-softmmu" that moment.

Regards,
-Gonglei

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Markus Armbruster]

Gonglei <arei.gonglei@huawei.com> writes:

> On 2015/3/4 21:16, Michael Tokarev wrote:
[...]
>> And a much more general solution is to actually test
>> patches before submitting them.  You obviously did not
>> test this series, having 3 errors in 9 patches, ie,
>> 1/3 of your patches does not work...
>> 
> Apologize earnestly for troubling you. :(
>
> I just thought they are so trivial, meanwhile I hadn't arm compiling
> environment (lack of
> libfdt-dev), and assigned "--target-list=x86_64-softmmu" that moment.

Hard-won personal experience: the more trivial a change is, the more
likely I am to screw it up in some stupid way.

In other words: no change is too trivial to screw it up.

Corollary: no change is trivial enough to skip testing.

Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/9] arm: fix memory leak

[Gonglei]

On 2015/3/5 16:47, Markus Armbruster wrote:
> Hard-won personal experience: the more trivial a change is, the more
> likely I am to screw it up in some stupid way.
> 
> In other words: no change is too trivial to screw it up.
> 
> Corollary: no change is trivial enough to skip testing.
I can't agree any more.    Never again. :(

Regards,
-Gonglei