[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] vl: fix resource leak with monitor_fdset_add_fd |
Date: |
Sat, 14 Mar 2015 09:08:36 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>> v1->v2: line length [Fam], pass &error_abort [Shannon]
>> ---
>> vl.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Eric Blake <address@hidden>
>
>> 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.