Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Pierre Morel]

On 2020-08-14 18:02, Pan Nengyuan wrote:
> Missing g_error_free() in vfio_ap_get_group() error path. Fix that.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: qemu-s390x@nongnu.org
> ---
>   hw/vfio/ap.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index b9330a8e6f..cec6fe1599 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -71,6 +71,7 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>       if (!group_path) {
>           error_setg(errp, "%s: no iommu_group found for %s: %s",
>                      VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev, gerror->message);
> +        g_error_free(gerror);
>           return NULL;
>       }
>   
> 

Thanks Pan,

Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>


-- 
Pierre Morel
IBM Lab Boeblingen

Re: [PATCH 12/12] test-util-sockets: Fix a memleak in test_socket_unix_abstract_good

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:18写道：
>
> Fix a memleak in test_socket_unix_abstract_good().
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---

Hi Nengyuan,
I have sent this two month ago:
-->https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00809.html

seems the maintainer forget to push it to upstream.

Thanks,
Li Qiang


>  tests/test-util-sockets.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
> index 261dc48c03..5c4204a130 100644
> --- a/tests/test-util-sockets.c
> +++ b/tests/test-util-sockets.c
> @@ -312,6 +312,7 @@ static void test_socket_unix_abstract_good(void)
>      g_thread_join(cli);
>      g_thread_join(serv);
>
> +    g_rand_free(r);
>      g_free(abstract_sock_name);
>  }
>  #endif
> --
> 2.18.2
>
>

[PATCH 00/12] fix some error memleaks

[Pan Nengyuan]

This series fix some Error/GError memleaks.

Pan Nengyuan (12):
  qga/channel-posix: Plug memory leak in ga_channel_write_all()
  hw/vfio/ap: Plug memleak in vfio_ap_get_group()
  elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init
  elf2dmp/pdb: Plug memleak in pdb_init_from_file
  target/i386/sev: Plug memleak in sev_read_file_base64
  ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
  target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
  migration/colo: Plug memleaks in colo_process_incoming_thread
  blockdev: Fix a memleak in drive_backup_prepare()
  block/file-posix: fix a possible undefined behavior
  vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string
  test-util-sockets: Fix a memleak in test_socket_unix_abstract_good

 block/file-posix.c         | 2 +-
 blockdev.c                 | 1 +
 contrib/elf2dmp/pdb.c      | 1 +
 contrib/elf2dmp/qemu_elf.c | 1 +
 hw/vfio/ap.c               | 1 +
 migration/colo.c           | 2 ++
 qga/channel-posix.c        | 6 +++++-
 target/i386/cpu.c          | 1 +
 target/i386/sev.c          | 1 +
 tests/test-util-sockets.c  | 1 +
 ui/gtk-gl-area.c           | 5 ++---
 ui/vnc-auth-sasl.c         | 1 +
 12 files changed, 18 insertions(+), 5 deletions(-)

-- 
2.18.2

[PATCH 01/12] qga/channel-posix: Plug memory leak in ga_channel_write_all()

[Pan Nengyuan]

Missing g_error_free on error path in ga_channel_write_all(). Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/channel-posix.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 8fc205ad21..0b151cb87b 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -249,7 +249,7 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
             buf += written;
         } else if (status != G_IO_STATUS_AGAIN) {
             g_warning("error writing to channel: %s", err->message);
-            return status;
+            goto out;
         }
     }
 
@@ -261,6 +261,10 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
         g_warning("error flushing channel: %s", err->message);
     }
 
+out:
+    if (err) {
+        g_error_free(err);
+    }
     return status;
 }
 
-- 
2.18.2

[PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Pan Nengyuan]

Missing g_error_free() in vfio_ap_get_group() error path. Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Tony Krowiak <akrowiak@linux.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Pierre Morel <pmorel@linux.ibm.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: qemu-s390x@nongnu.org
---
 hw/vfio/ap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index b9330a8e6f..cec6fe1599 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -71,6 +71,7 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
     if (!group_path) {
         error_setg(errp, "%s: no iommu_group found for %s: %s",
                    VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev, gerror->message);
+        g_error_free(gerror);
         return NULL;
     }
 
-- 
2.18.2

[PATCH 03/12] elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init

[Pan Nengyuan]

Missing g_error_free in QEMU_Elf_init() error path. Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
---
 contrib/elf2dmp/qemu_elf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
index 0db7816586..b601b6d7ba 100644
--- a/contrib/elf2dmp/qemu_elf.c
+++ b/contrib/elf2dmp/qemu_elf.c
@@ -126,6 +126,7 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
     qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
     if (gerr) {
         eprintf("Failed to map ELF dump file \'%s\'\n", filename);
+        g_error_free(gerr);
         return 1;
     }
 
-- 
2.18.2

[PATCH 04/12] elf2dmp/pdb: Plug memleak in pdb_init_from_file

[Pan Nengyuan]

Missing g_error_free in pdb_init_from_file() error path. Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
---
 contrib/elf2dmp/pdb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
index a5bd40c99d..b3a6547068 100644
--- a/contrib/elf2dmp/pdb.c
+++ b/contrib/elf2dmp/pdb.c
@@ -285,6 +285,7 @@ int pdb_init_from_file(const char *name, struct pdb_reader *reader)
     reader->gmf = g_mapped_file_new(name, TRUE, &gerr);
     if (gerr) {
         eprintf("Failed to map PDB file \'%s\'\n", name);
+        g_error_free(gerr);
         return 1;
     }
 
-- 
2.18.2

[PATCH 05/12] target/i386/sev: Plug memleak in sev_read_file_base64

[Pan Nengyuan]

Missing g_error_free() in sev_read_file_base64() error path.
Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/sev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index c3ecf86704..de4818da6d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -500,6 +500,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
 
     if (!g_file_get_contents(filename, &base64, &sz, &error)) {
         error_report("failed to read '%s' (%s)", filename, error->message);
+        g_error_free(error);
         return -1;
     }
 
-- 
2.18.2

[PATCH 06/12] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Pan Nengyuan]

Receiving error in local variable err, and forgot to free it.
Considering that there is no place to deal with it. Clean up.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk-gl-area.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 85f9d14c51..c740a7eb14 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -142,15 +142,14 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
     VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
     GdkWindow *window;
     GdkGLContext *ctx;
-    GError *err = NULL;
 
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     window = gtk_widget_get_window(vc->gfx.drawing_area);
-    ctx = gdk_window_create_gl_context(window, &err);
+    ctx = gdk_window_create_gl_context(window, NULL);
     gdk_gl_context_set_required_version(ctx,
                                         params->major_ver,
                                         params->minor_ver);
-    gdk_gl_context_realize(ctx, &err);
+    gdk_gl_context_realize(ctx, NULL);
     return ctx;
 }
 
-- 
2.18.2

[PATCH 07/12] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features

[Pan Nengyuan]

'err' forgot to free in x86_cpu_class_check_missing_features error path.
Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..4678aac0b4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4872,6 +4872,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
         new->value = g_strdup("type");
         *next = new;
         next = &new->next;
+        error_free(err);
     }
 
     x86_cpu_filter_features(xc, false);
-- 
2.18.2

[PATCH 08/12] migration/colo: Plug memleaks in colo_process_incoming_thread

[Pan Nengyuan]

'local_err' forgot to free in colo_process_incoming_thread error path.
Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
---
 migration/colo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/colo.c b/migration/colo.c
index ea7d1e9d4e..17b5afc6b5 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -870,6 +870,7 @@ void *colo_process_incoming_thread(void *opaque)
     replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
     if (local_err) {
         qemu_mutex_unlock_iothread();
+        error_report_err(local_err);
         goto out;
     }
 #else
@@ -882,6 +883,7 @@ void *colo_process_incoming_thread(void *opaque)
     colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
                       &local_err);
     if (local_err) {
+        error_report_err(local_err);
         goto out;
     }
 
-- 
2.18.2

[PATCH 09/12] blockdev: Fix a memleak in drive_backup_prepare()

[Pan Nengyuan]

'local_err' seems forgot to propagate in error path, it'll cause
a memleak. Fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-block@nongnu.org
---
 blockdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockdev.c b/blockdev.c
index 3848a9c8ab..842ac289c1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1801,6 +1801,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
     if (set_backing_hd) {
         bdrv_set_backing_hd(target_bs, source, &local_err);
         if (local_err) {
+            error_propagate(errp, local_err);
             goto unref;
         }
     }
-- 
2.18.2

[PATCH 10/12] block/file-posix: fix a possible undefined behavior

[Pan Nengyuan]

local_err is not initialized to NULL, it will cause a assert error as below:
qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.

Fixes: c6447510690
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Aarushi Mehta <mehta.aaru20@gmail.com>
Cc: qemu-block@nongnu.org
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 9a00d4190a..697a7d9eea 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2113,7 +2113,7 @@ static void raw_aio_attach_aio_context(BlockDriverState *bs,
 #endif
 #ifdef CONFIG_LINUX_IO_URING
     if (s->use_linux_io_uring) {
-        Error *local_err;
+        Error *local_err = NULL;
         if (!aio_setup_linux_io_uring(new_context, &local_err)) {
             error_reportf_err(local_err, "Unable to use linux io_uring, "
                                          "falling back to thread pool: ");
-- 
2.18.2

[PATCH 11/12] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

[Pan Nengyuan]

'addr' forgot to free in vnc_socket_ip_addr_string error path. Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-auth-sasl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 7b2b09f242..0517b2ead9 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
 
     if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
         error_setg(errp, "Not an inet socket type");
+        qapi_free_SocketAddress(addr);
         return NULL;
     }
     ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
-- 
2.18.2

[PATCH 12/12] test-util-sockets: Fix a memleak in test_socket_unix_abstract_good

[Pan Nengyuan]

Fix a memleak in test_socket_unix_abstract_good().

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
 tests/test-util-sockets.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index 261dc48c03..5c4204a130 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -312,6 +312,7 @@ static void test_socket_unix_abstract_good(void)
     g_thread_join(cli);
     g_thread_join(serv);
 
+    g_rand_free(r);
     g_free(abstract_sock_name);
 }
 #endif
-- 
2.18.2

Re: [PATCH 12/12] test-util-sockets: Fix a memleak in test_socket_unix_abstract_good

[Pan Nengyuan]

On 2020/8/14 22:50, Li Qiang wrote:
> Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:18写道：
>>
>> Fix a memleak in test_socket_unix_abstract_good().
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
> 
> Hi Nengyuan,
> I have sent this two month ago:
> -->https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00809.html
> 
> seems the maintainer forget to push it to upstream.

Yes, it's the same. Let's ignore this one.

Thanks.

> 
> Thanks,
> Li Qiang
> 
> 
>>  tests/test-util-sockets.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
>> index 261dc48c03..5c4204a130 100644
>> --- a/tests/test-util-sockets.c
>> +++ b/tests/test-util-sockets.c
>> @@ -312,6 +312,7 @@ static void test_socket_unix_abstract_good(void)
>>      g_thread_join(cli);
>>      g_thread_join(serv);
>>
>> +    g_rand_free(r);
>>      g_free(abstract_sock_name);
>>  }
>>  #endif
>> --
>> 2.18.2
>>
>>

Re: [PATCH 10/12] block/file-posix: fix a possible undefined behavior

[Stefano Garzarella]

On Fri, Aug 14, 2020 at 12:02:39PM -0400, Pan Nengyuan wrote:
> local_err is not initialized to NULL, it will cause a assert error as below:
> qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
> 
> Fixes: c6447510690
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Aarushi Mehta <mehta.aaru20@gmail.com>
> Cc: qemu-block@nongnu.org
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano

> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 9a00d4190a..697a7d9eea 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2113,7 +2113,7 @@ static void raw_aio_attach_aio_context(BlockDriverState *bs,
>  #endif
>  #ifdef CONFIG_LINUX_IO_URING
>      if (s->use_linux_io_uring) {
> -        Error *local_err;
> +        Error *local_err = NULL;
>          if (!aio_setup_linux_io_uring(new_context, &local_err)) {
>              error_reportf_err(local_err, "Unable to use linux io_uring, "
>                                           "falling back to thread pool: ");
> -- 
> 2.18.2
> 
> 

Re: [PATCH 10/12] block/file-posix: fix a possible undefined behavior

[Kevin Wolf]

Am 14.08.2020 um 18:02 hat Pan Nengyuan geschrieben:
> local_err is not initialized to NULL, it will cause a assert error as below:
> qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
> 
> Fixes: c6447510690
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [PATCH 09/12] blockdev: Fix a memleak in drive_backup_prepare()

[Kevin Wolf]

Am 14.08.2020 um 18:02 hat Pan Nengyuan geschrieben:
> 'local_err' seems forgot to propagate in error path, it'll cause
> a memleak. Fix it.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

I wonder if using ERRP_GUARD() wouldn't simplify this. Anyway, the fix
looks correct:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Cornelia Huck]

On Fri, 14 Aug 2020 12:02:31 -0400
Pan Nengyuan <pannengyuan@huawei.com> wrote:

> Missing g_error_free() in vfio_ap_get_group() error path. Fix that.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: qemu-s390x@nongnu.org
> ---
>  hw/vfio/ap.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index b9330a8e6f..cec6fe1599 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -71,6 +71,7 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>      if (!group_path) {
>          error_setg(errp, "%s: no iommu_group found for %s: %s",
>                     VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev, gerror->message);
> +        g_error_free(gerror);
>          return NULL;
>      }
>  

Thanks, applied.

Re: [PATCH 03/12] elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init

[Viktor Prutyanov]

On Fri, 14 Aug 2020 12:02:32 -0400
Pan Nengyuan <pannengyuan@huawei.com> wrote:

> Missing g_error_free in QEMU_Elf_init() error path. Fix that.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
>  contrib/elf2dmp/qemu_elf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> index 0db7816586..b601b6d7ba 100644
> --- a/contrib/elf2dmp/qemu_elf.c
> +++ b/contrib/elf2dmp/qemu_elf.c
> @@ -126,6 +126,7 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char
> *filename) qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
>      if (gerr) {
>          eprintf("Failed to map ELF dump file \'%s\'\n", filename);
> +        g_error_free(gerr);
>          return 1;
>      }
>  

Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>

-- 
Viktor Prutyanov

Re: [PATCH 04/12] elf2dmp/pdb: Plug memleak in pdb_init_from_file

[Viktor Prutyanov]

On Fri, 14 Aug 2020 12:02:33 -0400
Pan Nengyuan <pannengyuan@huawei.com> wrote:

> Missing g_error_free in pdb_init_from_file() error path. Fix that.
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
>  contrib/elf2dmp/pdb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> index a5bd40c99d..b3a6547068 100644
> --- a/contrib/elf2dmp/pdb.c
> +++ b/contrib/elf2dmp/pdb.c
> @@ -285,6 +285,7 @@ int pdb_init_from_file(const char *name, struct
> pdb_reader *reader) reader->gmf = g_mapped_file_new(name, TRUE,
> &gerr); if (gerr) {
>          eprintf("Failed to map PDB file \'%s\'\n", name);
> +        g_error_free(gerr);
>          return 1;
>      }
>  

Reviewed-by: Viktor Prutyanov <viktor.prutyanov@phystech.edu>

-- 
Viktor Prutyanov

Re: [PATCH 00/12] fix some error memleaks

[Pan Nengyuan]

ping!

On 2020/8/15 0:02, Pan Nengyuan wrote:
> This series fix some Error/GError memleaks.
> 
> Pan Nengyuan (12):
>   qga/channel-posix: Plug memory leak in ga_channel_write_all()
>   hw/vfio/ap: Plug memleak in vfio_ap_get_group()
>   elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init
>   elf2dmp/pdb: Plug memleak in pdb_init_from_file
>   target/i386/sev: Plug memleak in sev_read_file_base64
>   ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
>   target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features
>   migration/colo: Plug memleaks in colo_process_incoming_thread
>   blockdev: Fix a memleak in drive_backup_prepare()
>   block/file-posix: fix a possible undefined behavior
>   vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string
>   test-util-sockets: Fix a memleak in test_socket_unix_abstract_good
> 
>  block/file-posix.c         | 2 +-
>  blockdev.c                 | 1 +
>  contrib/elf2dmp/pdb.c      | 1 +
>  contrib/elf2dmp/qemu_elf.c | 1 +
>  hw/vfio/ap.c               | 1 +
>  migration/colo.c           | 2 ++
>  qga/channel-posix.c        | 6 +++++-
>  target/i386/cpu.c          | 1 +
>  target/i386/sev.c          | 1 +
>  tests/test-util-sockets.c  | 1 +
>  ui/gtk-gl-area.c           | 5 ++---
>  ui/vnc-auth-sasl.c         | 1 +
>  12 files changed, 18 insertions(+), 5 deletions(-)
> 

Re: [PATCH 11/12] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:40写道：
>
> 'addr' forgot to free in vnc_socket_ip_addr_string error path. Fix that.

s/forgot/is forgot, I think the maintainer will do this minor adjustment.

.



>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc-auth-sasl.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index 7b2b09f242..0517b2ead9 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
>
>      if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
>          error_setg(errp, "Not an inet socket type");
> +        qapi_free_SocketAddress(addr);
>          return NULL;
>      }
>      ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
> --
> 2.18.2
>
>

Re: [PATCH 10/12] block/file-posix: fix a possible undefined behavior

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:32写道：
>
> local_err is not initialized to NULL, it will cause a assert error as below:
> qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
>
> Fixes: c6447510690
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Aarushi Mehta <mehta.aaru20@gmail.com>
> Cc: qemu-block@nongnu.org
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 9a00d4190a..697a7d9eea 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2113,7 +2113,7 @@ static void raw_aio_attach_aio_context(BlockDriverState *bs,
>  #endif
>  #ifdef CONFIG_LINUX_IO_URING
>      if (s->use_linux_io_uring) {
> -        Error *local_err;
> +        Error *local_err = NULL;
>          if (!aio_setup_linux_io_uring(new_context, &local_err)) {
>              error_reportf_err(local_err, "Unable to use linux io_uring, "
>                                           "falling back to thread pool: ");
> --
> 2.18.2
>
>

Re: [PATCH 09/12] blockdev: Fix a memleak in drive_backup_prepare()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:54写道：
>
> 'local_err' seems forgot to propagate in error path, it'll cause
> a memleak. Fix it.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: qemu-block@nongnu.org
> ---
>  blockdev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 3848a9c8ab..842ac289c1 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1801,6 +1801,7 @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
>      if (set_backing_hd) {
>          bdrv_set_backing_hd(target_bs, source, &local_err);
>          if (local_err) {
> +            error_propagate(errp, local_err);
>              goto unref;
>          }
>      }
> --
> 2.18.2
>
>

Re: [PATCH 05/12] target/i386/sev: Plug memleak in sev_read_file_base64

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:37写道：
>
> Missing g_error_free() in sev_read_file_base64() error path.
> Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/sev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index c3ecf86704..de4818da6d 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -500,6 +500,7 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
>
>      if (!g_file_get_contents(filename, &base64, &sz, &error)) {
>          error_report("failed to read '%s' (%s)", filename, error->message);
> +        g_error_free(error);
>          return -1;
>      }
>
> --
> 2.18.2
>
>

Re: [PATCH 04/12] elf2dmp/pdb: Plug memleak in pdb_init_from_file

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:51写道：
>
> Missing g_error_free in pdb_init_from_file() error path. Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
>  contrib/elf2dmp/pdb.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/elf2dmp/pdb.c b/contrib/elf2dmp/pdb.c
> index a5bd40c99d..b3a6547068 100644
> --- a/contrib/elf2dmp/pdb.c
> +++ b/contrib/elf2dmp/pdb.c
> @@ -285,6 +285,7 @@ int pdb_init_from_file(const char *name, struct pdb_reader *reader)
>      reader->gmf = g_mapped_file_new(name, TRUE, &gerr);
>      if (gerr) {
>          eprintf("Failed to map PDB file \'%s\'\n", name);
> +        g_error_free(gerr);
>          return 1;
>      }
>
> --
> 2.18.2
>
>

Re: [PATCH 03/12] elf2dmp/qemu_elf: Plug memleak in QEMU_Elf_init

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:28写道：
>
> Missing g_error_free in QEMU_Elf_init() error path. Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Viktor Prutyanov <viktor.prutyanov@phystech.edu>
> ---
>  contrib/elf2dmp/qemu_elf.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/elf2dmp/qemu_elf.c b/contrib/elf2dmp/qemu_elf.c
> index 0db7816586..b601b6d7ba 100644
> --- a/contrib/elf2dmp/qemu_elf.c
> +++ b/contrib/elf2dmp/qemu_elf.c
> @@ -126,6 +126,7 @@ int QEMU_Elf_init(QEMU_Elf *qe, const char *filename)
>      qe->gmf = g_mapped_file_new(filename, TRUE, &gerr);
>      if (gerr) {
>          eprintf("Failed to map ELF dump file \'%s\'\n", filename);
> +        g_error_free(gerr);
>          return 1;
>      }
>
> --
> 2.18.2
>
>

Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:29写道：
>
> Missing g_error_free() in vfio_ap_get_group() error path. Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

I see Cornelia Huck has merged this in his tree.
Don't know whether this series will go separate maintainer's tree or
as go only one maintainer's tree(maybe Paolo's misc tree?).


Thanks,
Li Qiang

> ---
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: qemu-s390x@nongnu.org
> ---
>  hw/vfio/ap.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index b9330a8e6f..cec6fe1599 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -71,6 +71,7 @@ static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>      if (!group_path) {
>          error_setg(errp, "%s: no iommu_group found for %s: %s",
>                     VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev, gerror->message);
> +        g_error_free(gerror);
>          return NULL;
>      }
>
> --
> 2.18.2
>
>

Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Cornelia Huck]

On Wed, 26 Aug 2020 19:03:37 +0800
Li Qiang <liq3ea@gmail.com> wrote:

> Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:29写道：
> >
> > Missing g_error_free() in vfio_ap_get_group() error path. Fix that.
> >
> > Reported-by: Euler Robot <euler.robot@huawei.com>
> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>  
> 
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> 
> I see Cornelia Huck has merged this in his tree.

s/his/her/ :)

> Don't know whether this series will go separate maintainer's tree or
> as go only one maintainer's tree(maybe Paolo's misc tree?).

Whatever works best, but I still plan to send a pull req this week to
flush out my queue.

Re: [PATCH 02/12] hw/vfio/ap: Plug memleak in vfio_ap_get_group()

[Li Qiang]

Cornelia Huck <cohuck@redhat.com> 于2020年8月26日周三 下午7:41写道：
>
> On Wed, 26 Aug 2020 19:03:37 +0800
> Li Qiang <liq3ea@gmail.com> wrote:
>
> > Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:29写道：
> > >
> > > Missing g_error_free() in vfio_ap_get_group() error path. Fix that.
> > >
> > > Reported-by: Euler Robot <euler.robot@huawei.com>
> > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> >
> > Reviewed-by: Li Qiang <liq3ea@gmail.com>
> >
> > I see Cornelia Huck has merged this in his tree.
>
> s/his/her/ :)

I'm so sorry for this mistake.

Thanks,
Li Qiang

>
> > Don't know whether this series will go separate maintainer's tree or
> > as go only one maintainer's tree(maybe Paolo's misc tree?).
>
> Whatever works best, but I still plan to send a pull req this week to
> flush out my queue.



>

Re: [PATCH 01/12] qga/channel-posix: Plug memory leak in ga_channel_write_all()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:30写道：
>
> Missing g_error_free on error path in ga_channel_write_all(). Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qga/channel-posix.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index 8fc205ad21..0b151cb87b 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -249,7 +249,7 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
>              buf += written;
>          } else if (status != G_IO_STATUS_AGAIN) {
>              g_warning("error writing to channel: %s", err->message);
> -            return status;
> +            goto out;
>          }
>      }
>
> @@ -261,6 +261,10 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
>          g_warning("error flushing channel: %s", err->message);
>      }
>
> +out:
> +    if (err) {
> +        g_error_free(err);
> +    }
>      return status;
>  }
>
> --
> 2.18.2
>
>

Re: [PATCH 06/12] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:15写道：
>
> Receiving error in local variable err, and forgot to free it.
> Considering that there is no place to deal with it. Clean up.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/gtk-gl-area.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 85f9d14c51..c740a7eb14 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -142,15 +142,14 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>      GdkWindow *window;
>      GdkGLContext *ctx;
> -    GError *err = NULL;
>
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>      window = gtk_widget_get_window(vc->gfx.drawing_area);
> -    ctx = gdk_window_create_gl_context(window, &err);
> +    ctx = gdk_window_create_gl_context(window, NULL);
>      gdk_gl_context_set_required_version(ctx,
>                                          params->major_ver,
>                                          params->minor_ver);
> -    gdk_gl_context_realize(ctx, &err);
> +    gdk_gl_context_realize(ctx, NULL);
>      return ctx;
>  }

Maybe we should check the return value of  'gdk_window_create_gl_context'
and 'gdk_gl_context_realize' instead of omitting it?

Thanks,
Li Qiang

>

> --
> 2.18.2
>
>

Re: [PATCH 07/12] target/i386/cpu: Fix memleak in x86_cpu_class_check_missing_features

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:38写道：
>
> 'err' forgot to free in x86_cpu_class_check_missing_features error path.
> Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 588f32e136..4678aac0b4 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4872,6 +4872,7 @@ static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>          new->value = g_strdup("type");
>          *next = new;
>          next = &new->next;
> +        error_free(err);
>      }
>
>      x86_cpu_filter_features(xc, false);
> --
> 2.18.2
>
>

Re: [PATCH 08/12] migration/colo: Plug memleaks in colo_process_incoming_thread

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:52写道：
>
> 'local_err' forgot to free in colo_process_incoming_thread error path.
> Fix that.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> ---
>  migration/colo.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/migration/colo.c b/migration/colo.c
> index ea7d1e9d4e..17b5afc6b5 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -870,6 +870,7 @@ void *colo_process_incoming_thread(void *opaque)
>      replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
>      if (local_err) {
>          qemu_mutex_unlock_iothread();
> +        error_report_err(local_err);
>          goto out;
>      }
>  #else
> @@ -882,6 +883,7 @@ void *colo_process_incoming_thread(void *opaque)
>      colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
>                        &local_err);
>      if (local_err) {
> +        error_report_err(local_err);
>          goto out;
>      }
>

Could we arrange 'error_report_err' in 'out' label?
Like this:

if (local_err) {
    error_report_err(local_err);
}

Thanks,
Li Qiang



> --
> 2.18.2
>
>

Re: [PATCH 08/12] migration/colo: Plug memleaks in colo_process_incoming_thread

[Pan Nengyuan]

On 2020/8/26 20:37, Li Qiang wrote:
> Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:52写道：
>>
>> 'local_err' forgot to free in colo_process_incoming_thread error path.
>> Fix that.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> ---
>>  migration/colo.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/migration/colo.c b/migration/colo.c
>> index ea7d1e9d4e..17b5afc6b5 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -870,6 +870,7 @@ void *colo_process_incoming_thread(void *opaque)
>>      replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
>>      if (local_err) {
>>          qemu_mutex_unlock_iothread();
>> +        error_report_err(local_err);
>>          goto out;
>>      }
>>  #else
>> @@ -882,6 +883,7 @@ void *colo_process_incoming_thread(void *opaque)
>>      colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
>>                        &local_err);
>>      if (local_err) {
>> +        error_report_err(local_err);
>>          goto out;
>>      }
>>
> 
> Could we arrange 'error_report_err' in 'out' label?
> Like this:
> 
> if (local_err) {
>     error_report_err(local_err);
> }

Similar to the other place in the same function, I didn't arrange them in 'out' label:

    while (mis->state == MIGRATION_STATUS_COLO) {
        colo_wait_handle_message(mis, fb, bioc, &local_err);
        if (local_err) {
            error_report_err(local_err);
            break;
        }

But I think it's a good idea to arrange them in 'out' label. I will change it.

Thanks.

> 
> Thanks,
> Li Qiang
> 
> 
> 
>> --
>> 2.18.2
>>
>>
> .
> 

Re: [PATCH 06/12] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Pan Nengyuan]

On 2020/8/26 20:20, Li Qiang wrote:
> Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:15写道：
>>
>> Receiving error in local variable err, and forgot to free it.
>> Considering that there is no place to deal with it. Clean up.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  ui/gtk-gl-area.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
>> index 85f9d14c51..c740a7eb14 100644
>> --- a/ui/gtk-gl-area.c
>> +++ b/ui/gtk-gl-area.c
>> @@ -142,15 +142,14 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
>>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
>>      GdkWindow *window;
>>      GdkGLContext *ctx;
>> -    GError *err = NULL;
>>
>>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>>      window = gtk_widget_get_window(vc->gfx.drawing_area);
>> -    ctx = gdk_window_create_gl_context(window, &err);
>> +    ctx = gdk_window_create_gl_context(window, NULL);
>>      gdk_gl_context_set_required_version(ctx,
>>                                          params->major_ver,
>>                                          params->minor_ver);
>> -    gdk_gl_context_realize(ctx, &err);
>> +    gdk_gl_context_realize(ctx, NULL);
>>      return ctx;
>>  }
> 
> Maybe we should check the return value of  'gdk_window_create_gl_context'
> and 'gdk_gl_context_realize' instead of omitting it?

OK, Agree with you.

How about check the value like the below?
(Return NULL when error happens in gdk_gl_context_realize. It's different from the original.)

Thanks.

--------
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 85f9d14c51..98c22d23f5 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -147,10 +147,21 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     window = gtk_widget_get_window(vc->gfx.drawing_area);
     ctx = gdk_window_create_gl_context(window, &err);
+    if (err) {
+        g_printerr("Create gdk gl context failed: %s\n", err->message);
+        g_error_free(err);
+        return NULL;
+    }
     gdk_gl_context_set_required_version(ctx,
                                         params->major_ver,
                                         params->minor_ver);
     gdk_gl_context_realize(ctx, &err);
+    if (err) {
+        g_printerr("Realize gdk gl context failed: %s\n", err->message);
+        g_error_free(err);
+        g_clear_object(&ctx);
+        return NULL;
+    }
     return ctx;
 }


> 
> Thanks,
> Li Qiang
> 
>>
> 
>> --
>> 2.18.2
>>
>>
> .
> 

Re: [PATCH 06/12] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Li Qiang]

Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月27日周四 下午3:06写道：
>
>
>
> On 2020/8/26 20:20, Li Qiang wrote:
> > Pan Nengyuan <pannengyuan@huawei.com> 于2020年8月14日周五 下午6:15写道：
> >>
> >> Receiving error in local variable err, and forgot to free it.
> >> Considering that there is no place to deal with it. Clean up.
> >>
> >> Reported-by: Euler Robot <euler.robot@huawei.com>
> >> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> >> ---
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> ---
> >>  ui/gtk-gl-area.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> >> index 85f9d14c51..c740a7eb14 100644
> >> --- a/ui/gtk-gl-area.c
> >> +++ b/ui/gtk-gl-area.c
> >> @@ -142,15 +142,14 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
> >>      VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
> >>      GdkWindow *window;
> >>      GdkGLContext *ctx;
> >> -    GError *err = NULL;
> >>
> >>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
> >>      window = gtk_widget_get_window(vc->gfx.drawing_area);
> >> -    ctx = gdk_window_create_gl_context(window, &err);
> >> +    ctx = gdk_window_create_gl_context(window, NULL);
> >>      gdk_gl_context_set_required_version(ctx,
> >>                                          params->major_ver,
> >>                                          params->minor_ver);
> >> -    gdk_gl_context_realize(ctx, &err);
> >> +    gdk_gl_context_realize(ctx, NULL);
> >>      return ctx;
> >>  }
> >
> > Maybe we should check the return value of  'gdk_window_create_gl_context'
> > and 'gdk_gl_context_realize' instead of omitting it?
>
> OK, Agree with you.
>
> How about check the value like the below?

I think it is OK.

> (Return NULL when error happens in gdk_gl_context_realize. It's different from the original.)

Don't familiar with the internal of how gtk-gl work.

Maybe you can wait for other review or Gerd's decision.

Thanks,
Li Qiang

>
> Thanks.
>
> --------
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 85f9d14c51..98c22d23f5 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -147,10 +147,21 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
>      gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
>      window = gtk_widget_get_window(vc->gfx.drawing_area);
>      ctx = gdk_window_create_gl_context(window, &err);
> +    if (err) {
> +        g_printerr("Create gdk gl context failed: %s\n", err->message);
> +        g_error_free(err);
> +        return NULL;
> +    }
>      gdk_gl_context_set_required_version(ctx,
>                                          params->major_ver,
>                                          params->minor_ver);
>      gdk_gl_context_realize(ctx, &err);
> +    if (err) {
> +        g_printerr("Realize gdk gl context failed: %s\n", err->message);
> +        g_error_free(err);
> +        g_clear_object(&ctx);
> +        return NULL;
> +    }
>      return ctx;
>  }
>
>
> >
> > Thanks,
> > Li Qiang
> >
> >>
> >
> >> --
> >> 2.18.2
> >>
> >>
> > .
> >
>