Re: [PATCH v4] migration: Plug memory leak with migration URIs

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Wed, Nov 29, 2023 at 08:43:01PM +0000, Het Gala wrote:
>> migrate_uri_parse() allocates memory to 'channel' if the user
>> opts for old syntax - uri, which is leaked because there is no
>> code for freeing 'channel'.
>> So, free channel to avoid memory leak in case where 'channels'
>> is empty and uri parsing is required.
>> 
>> Fixes: 5994024f ("migration: Implement MigrateChannelList to qmp migration 
>> flow")
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
>> @@ -533,18 +533,18 @@ static void qemu_start_incoming_migration(const char 
>> *uri, bool has_channels,
>>              error_setg(errp, "Channel list has more than one entries");
>>              return;
>>          }
>> -        channel = channels->value;
>> +        addr = channels->value->addr;
>>      } else if (uri) {
>>          /* caller uses the old URI syntax */
>>          if (!migrate_uri_parse(uri, &channel, errp)) {
>>              return;
>>          }
>> +        addr = channel->addr;
>>      } else {
>>          error_setg(errp, "neither 'uri' or 'channels' argument are "
>>                     "specified in 'migrate-incoming' qmp command ");
>>          return;
>>      }
>> -    addr = channel->addr;
>
> Why these "addr" lines need change?  Won't that behave the same as before?

In the first case, @channel is now null.  If we left the assignment to
@addr alone, it would crash.  Clearer now?