All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-trivial] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-12 14:57 Paolo Bonzini
  2015-03-19  8:22 ` Michael Tokarev
  0 siblings, 1 reply; 10+ messages in thread
From: Paolo Bonzini @ 2015-03-12 14:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial

monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
command add_fd).  Free it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index eba5d4c..8902435 100644
--- a/vl.c
+++ b/vl.c
@@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     int fd, dupfd, flags;
     int64_t fdset_id;
     const char *fd_opaque = NULL;
+    AddfdInfo *fdinfo;
 
     fd = qemu_opt_get_number(opts, "fd", -1);
     fdset_id = qemu_opt_get_number(opts, "set", -1);
@@ -1060,8 +1061,9 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     }
 
     /* add the duplicate fd, and optionally the opaque string, to the fd set */
-    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
-                         fd_opaque, NULL);
+    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
+                                  fd_opaque, NULL);
+    g_free(fdinfo);
 
     return 0;
 }
-- 
2.3.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-trivial] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-13 12:55 ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2015-03-13 12:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, famz, zhaoshenglong

monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
command add_fd).  Free it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v1->v2: line length [Fam], pass &error_abort [Shannon]
---
 vl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index eba5d4c..9eae8f9 100644
--- a/vl.c
+++ b/vl.c
@@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     int fd, dupfd, flags;
     int64_t fdset_id;
     const char *fd_opaque = NULL;
+    AddfdInfo *fdinfo;
 
     fd = qemu_opt_get_number(opts, "fd", -1);
     fdset_id = qemu_opt_get_number(opts, "set", -1);
@@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     }
 
     /* add the duplicate fd, and optionally the opaque string, to the fd set */
-    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
-                         fd_opaque, NULL);
+    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
+                                  fd_opaque ? true : false, fd_opaque,
+                                  &error_abort);
+    g_free(fdinfo);
 
     return 0;
 }
-- 
2.3.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-13 12:55 ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2015-03-13 12:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, famz, zhaoshenglong

monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
command add_fd).  Free it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v1->v2: line length [Fam], pass &error_abort [Shannon]
---
 vl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index eba5d4c..9eae8f9 100644
--- a/vl.c
+++ b/vl.c
@@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     int fd, dupfd, flags;
     int64_t fdset_id;
     const char *fd_opaque = NULL;
+    AddfdInfo *fdinfo;
 
     fd = qemu_opt_get_number(opts, "fd", -1);
     fdset_id = qemu_opt_get_number(opts, "set", -1);
@@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
     }
 
     /* add the duplicate fd, and optionally the opaque string, to the fd set */
-    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
-                         fd_opaque, NULL);
+    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
+                                  fd_opaque ? true : false, fd_opaque,
+                                  &error_abort);
+    g_free(fdinfo);
 
     return 0;
 }
-- 
2.3.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-trivial] [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
  2015-03-13 12:55 ` [Qemu-devel] " Paolo Bonzini
@ 2015-03-13 13:56   ` Eric Blake
  -1 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2015-03-13 13:56 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, famz, zhaoshenglong

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On 03/13/2015 06:55 AM, Paolo Bonzini wrote:
> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
> command add_fd).  Free it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
> ---
>  vl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/vl.c b/vl.c
> index eba5d4c..9eae8f9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      int fd, dupfd, flags;
>      int64_t fdset_id;
>      const char *fd_opaque = NULL;
> +    AddfdInfo *fdinfo;
>  
>      fd = qemu_opt_get_number(opts, "fd", -1);
>      fdset_id = qemu_opt_get_number(opts, "set", -1);
> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      }
>  
>      /* add the duplicate fd, and optionally the opaque string, to the fd set */
> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
> -                         fd_opaque, NULL);
> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
> +                                  fd_opaque ? true : false, fd_opaque,

I might have written !!fd_opaque (as cond ? true : false always looks so
long), but that's cosmetic and doesn't affect the review.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-13 13:56   ` Eric Blake
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2015-03-13 13:56 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, famz, zhaoshenglong

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On 03/13/2015 06:55 AM, Paolo Bonzini wrote:
> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
> command add_fd).  Free it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
> ---
>  vl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/vl.c b/vl.c
> index eba5d4c..9eae8f9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      int fd, dupfd, flags;
>      int64_t fdset_id;
>      const char *fd_opaque = NULL;
> +    AddfdInfo *fdinfo;
>  
>      fd = qemu_opt_get_number(opts, "fd", -1);
>      fdset_id = qemu_opt_get_number(opts, "set", -1);
> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      }
>  
>      /* add the duplicate fd, and optionally the opaque string, to the fd set */
> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
> -                         fd_opaque, NULL);
> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
> +                                  fd_opaque ? true : false, fd_opaque,

I might have written !!fd_opaque (as cond ? true : false always looks so
long), but that's cosmetic and doesn't affect the review.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-trivial] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
  2015-03-13 12:55 ` [Qemu-devel] " Paolo Bonzini
@ 2015-03-14  1:15   ` Shannon Zhao
  -1 siblings, 0 replies; 10+ messages in thread
From: Shannon Zhao @ 2015-03-14  1:15 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, famz, Huangpeng (Peter)

On 2015/3/13 20:55, Paolo Bonzini wrote:
> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
> command add_fd).  Free it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
> ---
>  vl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

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

> diff --git a/vl.c b/vl.c
> index eba5d4c..9eae8f9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      int fd, dupfd, flags;
>      int64_t fdset_id;
>      const char *fd_opaque = NULL;
> +    AddfdInfo *fdinfo;
>  
>      fd = qemu_opt_get_number(opts, "fd", -1);
>      fdset_id = qemu_opt_get_number(opts, "set", -1);
> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      }
>  
>      /* add the duplicate fd, and optionally the opaque string, to the fd set */
> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
> -                         fd_opaque, NULL);
> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
> +                                  fd_opaque ? true : false, fd_opaque,
> +                                  &error_abort);
> +    g_free(fdinfo);
>  
>      return 0;
>  }
> 


-- 
Thanks,
Shannon



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-14  1:15   ` Shannon Zhao
  0 siblings, 0 replies; 10+ messages in thread
From: Shannon Zhao @ 2015-03-14  1:15 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial, famz, Huangpeng (Peter)

On 2015/3/13 20:55, Paolo Bonzini wrote:
> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
> command add_fd).  Free it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
> ---
>  vl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

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

> diff --git a/vl.c b/vl.c
> index eba5d4c..9eae8f9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      int fd, dupfd, flags;
>      int64_t fdset_id;
>      const char *fd_opaque = NULL;
> +    AddfdInfo *fdinfo;
>  
>      fd = qemu_opt_get_number(opts, "fd", -1);
>      fdset_id = qemu_opt_get_number(opts, "set", -1);
> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>      }
>  
>      /* add the duplicate fd, and optionally the opaque string, to the fd set */
> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
> -                         fd_opaque, NULL);
> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
> +                                  fd_opaque ? true : false, fd_opaque,
> +                                  &error_abort);
> +    g_free(fdinfo);
>  
>      return 0;
>  }
> 


-- 
Thanks,
Shannon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-trivial] [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
  2015-03-13 13:56   ` Eric Blake
@ 2015-03-14  8:08     ` Markus Armbruster
  -1 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-03-14  8:08 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-trivial, Paolo Bonzini, famz, qemu-devel, zhaoshenglong

Eric Blake <eblake@redhat.com> writes:

> On 03/13/2015 06:55 AM, Paolo Bonzini wrote:
>> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
>> command add_fd).  Free it.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
>> ---
>>  vl.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> diff --git a/vl.c b/vl.c
>> index eba5d4c..9eae8f9 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>      int fd, dupfd, flags;
>>      int64_t fdset_id;
>>      const char *fd_opaque = NULL;
>> +    AddfdInfo *fdinfo;
>>  
>>      fd = qemu_opt_get_number(opts, "fd", -1);
>>      fdset_id = qemu_opt_get_number(opts, "set", -1);
>> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>      }
>>  
>>      /* add the duplicate fd, and optionally the opaque string, to
>> the fd set */
>> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
>> -                         fd_opaque, NULL);
>> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
>> +                                  fd_opaque ? true : false, fd_opaque,
>
> I might have written !!fd_opaque (as cond ? true : false always looks so
> long), but that's cosmetic and doesn't affect the review.

The use of ?: here borders on code obfuscation.  Please clean it up.

If you can't stand !!fd_opaque, then use fd_opaque != NULL.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
@ 2015-03-14  8:08     ` Markus Armbruster
  0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2015-03-14  8:08 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-trivial, Paolo Bonzini, famz, qemu-devel, zhaoshenglong

Eric Blake <eblake@redhat.com> writes:

> On 03/13/2015 06:55 AM, Paolo Bonzini wrote:
>> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
>> command add_fd).  Free it.
>> 
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> 	v1->v2: line length [Fam], pass &error_abort [Shannon]
>> ---
>>  vl.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> diff --git a/vl.c b/vl.c
>> index eba5d4c..9eae8f9 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1011,6 +1011,7 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>      int fd, dupfd, flags;
>>      int64_t fdset_id;
>>      const char *fd_opaque = NULL;
>> +    AddfdInfo *fdinfo;
>>  
>>      fd = qemu_opt_get_number(opts, "fd", -1);
>>      fdset_id = qemu_opt_get_number(opts, "set", -1);
>> @@ -1060,8 +1061,10 @@ static int parse_add_fd(QemuOpts *opts, void *opaque)
>>      }
>>  
>>      /* add the duplicate fd, and optionally the opaque string, to
>> the fd set */
>> -    monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
>> -                         fd_opaque, NULL);
>> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
>> +                                  fd_opaque ? true : false, fd_opaque,
>
> I might have written !!fd_opaque (as cond ? true : false always looks so
> long), but that's cosmetic and doesn't affect the review.

The use of ?: here borders on code obfuscation.  Please clean it up.

If you can't stand !!fd_opaque, then use fd_opaque != NULL.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-trivial] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
  2015-03-12 14:57 [Qemu-trivial] " Paolo Bonzini
@ 2015-03-19  8:22 ` Michael Tokarev
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Tokarev @ 2015-03-19  8:22 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: qemu-trivial

12.03.2015 17:57, Paolo Bonzini wrote:
> monitor_fdset_add_fd returns an AddfdInfo struct (used by the QMP
> command add_fd).  Free it.

Applied to -trivial, with a tiny change:

> +    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id, fd_opaque ? true : false,
> +                                  fd_opaque, NULL);

    fdinfo = monitor_fdset_add_fd(dupfd, true, fdset_id,
                                  fd_opaque ? true : false, fd_opaque, NULL);

(this fits in 80 chars a line).  Another variant is to use `fd_opaque != NULL' here
instead of this ?true:false construct.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-19  8:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-13 12:55 [Qemu-trivial] [PATCH] vl: fix resource leak with monitor_fdset_add_fd Paolo Bonzini
2015-03-13 12:55 ` [Qemu-devel] " Paolo Bonzini
2015-03-13 13:56 ` [Qemu-trivial] " Eric Blake
2015-03-13 13:56   ` Eric Blake
2015-03-14  8:08   ` [Qemu-trivial] " Markus Armbruster
2015-03-14  8:08     ` Markus Armbruster
2015-03-14  1:15 ` [Qemu-trivial] " Shannon Zhao
2015-03-14  1:15   ` [Qemu-devel] " Shannon Zhao
  -- strict thread matches above, loose matches on Subject: below --
2015-03-12 14:57 [Qemu-trivial] " Paolo Bonzini
2015-03-19  8:22 ` Michael Tokarev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.