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

[Markus Armbruster]

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.