Re: [Qemu-trivial] [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd

[Shannon Zhao <zhaoshenglong@huawei.com>]

On 2015/3/12 22:57, 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>
> ---
>  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;
>  }
> 

Hi Paolo,

Not about the resource leak, but I think we should check fdinfo == NULL and return -1.
Then when error occurs, qemu will exit. Besides we should pass a meaningful errp not NULL.
So qemu can report the error reason. Right?

-- 
Thanks,
Shannon