Markus Armbruster <arm...@redhat.com> writes:
> Eric Blake <ebl...@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 <arm...@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.
