Re: [Qemu-devel] [PATCH 6/7] sockets: Limit SocketAddressLegacy except to external interfaces

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 04/26/2017 02:36 AM, Markus Armbruster wrote:
>>
>> I think it reads fine if you shorten the subject via s/except //
>
> It doesn't unless.  Will fix, thanks!
>
>>> SocketAddressLegacy is a simple union, and simple unions are awkward:
>>> they have their variant members wrapped in a "data" object on the
>>> wire, and require additional indirections in C.  SocketAddress is the
>>> equivalent flat union.  Convert all users of SocketAddressLegacy to
>>> SocketAddress, except for existing external interfaces.
>>> 
>>> See also commit fce5d53..9445673 and 85a82e8..c5f1ae3.
>>> 
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>
>>> +++ b/blockdev-nbd.c
>>> @@ -99,9 +99,8 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
>>>  }
>>>  
>>>  
>>> -void qmp_nbd_server_start(SocketAddressLegacy *addr,
>>> -                          bool has_tls_creds, const char *tls_creds,
>>> -                          Error **errp)
>>> +void nbd_server_start(SocketAddress *addr, const char *tls_creds,
>>> +                      Error **errp)
>>>  {
>> ...
>>> @@ -145,6 +144,16 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
>>>      nbd_server = NULL;
>>>  }
>>>  
>>> +void qmp_nbd_server_start(SocketAddressLegacy *addr,
>>> +                          bool has_tls_creds, const char *tls_creds,
>>> +                          Error **errp)
>>> +{
>>> +    SocketAddress *addr_flat = socket_address_flatten(addr);
>>> +
>>> +    nbd_server_start(addr_flat, tls_creds, errp);
>>
>> We didn't always guarantee that tls_creds was initialized when
>> has_tls_creds was false, but it's been quite some time that we fixed
>> that.  So what you have works, and I'm not sure if it's worth using the
>> longer:
>> has_tls_creds ? tls_creds : NULL

Forgot to fill in the blank here: the conditional operator adds neither
readability nor robustness.  All it adds is noise.

I still hope to get rid of the has_ flag for pointers.