Re: [Qemu-devel] [PATCH] migration: Fix seg with missing port

[Eric Blake]

On 09/12/2016 02:11 PM, Daniel P. Berrange wrote:
> On Mon, Sep 12, 2016 at 08:03:54PM +0100, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <address@hidden>
>>
>> The command :
>>    migrate tcp:localhost:
>>
>>    currently segs; fix it so it now says:
>>
>>    error parsing address 'localhost:'
>>
>> and the same for -incoming.
>>
>> (We know that errp is non-null; callers use a local_err).
> 
> I'd still be more comfortable with us using a local Error
> object here, as it illustrates best practice and protects
> against future accidents.

Concur. Having an example contrary to best practice makes it far too
easy for that example to be copied somewhere where it will later break.

>> +++ b/migration/socket.c
>> @@ -113,7 +113,9 @@ void tcp_start_outgoing_migration(MigrationState *s,
>>                                    Error **errp)
>>  {
>>      SocketAddress *saddr = tcp_build_address(host_port, errp);
>> -    socket_start_outgoing_migration(s, saddr, errp);
>> +    if (!*errp) {
>> +        socket_start_outgoing_migration(s, saddr, errp);
>> +    }

Since we make a decision about code flow based on whether an earlier
error occurred, this SHOULD be:

{
    Error *err = NULL;
    SocketAddress *saddr = tcp_build_address(host_port, &err);

    if (!err) {
        socket_start_outgoing_migration(s, saddr, &err);
    }
    error_propagate(errp, err);
}

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

signature.asc
Description: OpenPGP digital signature