Re: [Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-19 Thread Stefan Hajnoczi
On Mon, Nov 18, 2013 at 05:20:41PM +0200, Michael S. Tsirkin wrote:
 On Mon, Nov 18, 2013 at 04:14:08PM +0100, Stefan Hajnoczi wrote:
  On Mon, Nov 18, 2013 at 09:47:25PM +0800, Amos Kong wrote:
   object_get_canonical_path() returns a gchar*, it should be freeed by the
   caller.
   
   Signed-off-by: Amos Kong ak...@redhat.com
   ---
hw/net/virtio-net.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)
  
  Thanks, applied to my net tree (will send pull request for QEMU 1.7):
  https://github.com/stefanha/qemu/commits/net
  
  Stefan
 
 I'd prefer it for my comment to be addressed first ...

I folded your suggestion into the commit.

Stefan



[Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-18 Thread Amos Kong
object_get_canonical_path() returns a gchar*, it should be freeed by the
caller.

Signed-off-by: Amos Kong ak...@redhat.com
---
 hw/net/virtio-net.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 613f144..2b2fb57 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -198,15 +198,14 @@ static void rxfilter_notify(NetClientState *nc)
 {
 QObject *event_data;
 VirtIONet *n = qemu_get_nic_opaque(nc);
+gchar *path = object_get_canonical_path(OBJECT(n-qdev));
 
 if (nc-rxfilter_notify_enabled) {
 if (n-netclient_name) {
 event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
-n-netclient_name,
-
object_get_canonical_path(OBJECT(n-qdev)));
+n-netclient_name, path);
 } else {
-event_data = qobject_from_jsonf({ 'path': %s },
-
object_get_canonical_path(OBJECT(n-qdev)));
+event_data = qobject_from_jsonf({ 'path': %s }, path);
 }
 monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
 qobject_decref(event_data);
@@ -214,6 +213,7 @@ static void rxfilter_notify(NetClientState *nc)
 /* disable event notification to avoid events flooding */
 nc-rxfilter_notify_enabled = 0;
 }
+g_free(path);
 }
 
 static char *mac_strdup_printf(const uint8_t *mac)
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-18 Thread Michael S. Tsirkin
On Mon, Nov 18, 2013 at 09:47:25PM +0800, Amos Kong wrote:
 object_get_canonical_path() returns a gchar*, it should be freeed by the
 caller.
 
 Signed-off-by: Amos Kong ak...@redhat.com

Reviewed-by: Michael S. Tsirkin m...@redhat.com

 ---
  hw/net/virtio-net.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index 613f144..2b2fb57 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -198,15 +198,14 @@ static void rxfilter_notify(NetClientState *nc)
  {
  QObject *event_data;
  VirtIONet *n = qemu_get_nic_opaque(nc);
 +gchar *path = object_get_canonical_path(OBJECT(n-qdev));
  
  if (nc-rxfilter_notify_enabled) {

It would be a bit nicer to put gchar *path within this scope.

  if (n-netclient_name) {
  event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
 -n-netclient_name,
 -
 object_get_canonical_path(OBJECT(n-qdev)));
 +n-netclient_name, path);
  } else {
 -event_data = qobject_from_jsonf({ 'path': %s },
 -
 object_get_canonical_path(OBJECT(n-qdev)));
 +event_data = qobject_from_jsonf({ 'path': %s }, path);
  }
  monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
  qobject_decref(event_data);
 @@ -214,6 +213,7 @@ static void rxfilter_notify(NetClientState *nc)
  /* disable event notification to avoid events flooding */
  nc-rxfilter_notify_enabled = 0;
  }
 +g_free(path);
  }
  
  static char *mac_strdup_printf(const uint8_t *mac)
 -- 
 1.8.3.1



Re: [Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-18 Thread Vlad Yasevich
On 11/18/2013 08:47 AM, Amos Kong wrote:
 object_get_canonical_path() returns a gchar*, it should be freeed by the
 caller.
 
 Signed-off-by: Amos Kong ak...@redhat.com

Reviewed-by: Vlad Yasevich vyase...@redhat.com

-vlad

 ---
  hw/net/virtio-net.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
 index 613f144..2b2fb57 100644
 --- a/hw/net/virtio-net.c
 +++ b/hw/net/virtio-net.c
 @@ -198,15 +198,14 @@ static void rxfilter_notify(NetClientState *nc)
  {
  QObject *event_data;
  VirtIONet *n = qemu_get_nic_opaque(nc);
 +gchar *path = object_get_canonical_path(OBJECT(n-qdev));
  
  if (nc-rxfilter_notify_enabled) {
  if (n-netclient_name) {
  event_data = qobject_from_jsonf({ 'name': %s, 'path': %s },
 -n-netclient_name,
 -
 object_get_canonical_path(OBJECT(n-qdev)));
 +n-netclient_name, path);
  } else {
 -event_data = qobject_from_jsonf({ 'path': %s },
 -
 object_get_canonical_path(OBJECT(n-qdev)));
 +event_data = qobject_from_jsonf({ 'path': %s }, path);
  }
  monitor_protocol_event(QEVENT_NIC_RX_FILTER_CHANGED, event_data);
  qobject_decref(event_data);
 @@ -214,6 +213,7 @@ static void rxfilter_notify(NetClientState *nc)
  /* disable event notification to avoid events flooding */
  nc-rxfilter_notify_enabled = 0;
  }
 +g_free(path);
  }
  
  static char *mac_strdup_printf(const uint8_t *mac)
 




Re: [Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-18 Thread Stefan Hajnoczi
On Mon, Nov 18, 2013 at 09:47:25PM +0800, Amos Kong wrote:
 object_get_canonical_path() returns a gchar*, it should be freeed by the
 caller.
 
 Signed-off-by: Amos Kong ak...@redhat.com
 ---
  hw/net/virtio-net.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

Thanks, applied to my net tree (will send pull request for QEMU 1.7):
https://github.com/stefanha/qemu/commits/net

Stefan



Re: [Qemu-devel] [PATCH] virtio-net: fix the memory leak in rxfilter_notify()

2013-11-18 Thread Michael S. Tsirkin
On Mon, Nov 18, 2013 at 04:14:08PM +0100, Stefan Hajnoczi wrote:
 On Mon, Nov 18, 2013 at 09:47:25PM +0800, Amos Kong wrote:
  object_get_canonical_path() returns a gchar*, it should be freeed by the
  caller.
  
  Signed-off-by: Amos Kong ak...@redhat.com
  ---
   hw/net/virtio-net.c | 8 
   1 file changed, 4 insertions(+), 4 deletions(-)
 
 Thanks, applied to my net tree (will send pull request for QEMU 1.7):
 https://github.com/stefanha/qemu/commits/net
 
 Stefan

I'd prefer it for my comment to be addressed first ...