On 10/20/2017 02:30 PM, Vladimir Sementsov-Ogievskiy wrote:
>>>> + } else if (client->structured_reply) {
>>>> + ret = nbd_negotiate_send_rep_err(
>>>> + client->ioc, NBD_REP_ERR_INVALID, option,
>>>> errp,
>>>> + "structured reply already negotiated");
>>>> + } else {
>>>> + ret = nbd_negotiate_send_rep(client->ioc,
>>>> NBD_REP_ACK,
>>>> + option, errp);
>>>> + }
>>> you've dropped "if (ret < 0) { return ret }", but the following two
>>> lines should not be
>>> executed if ret < 0.. May be it doesn't matter (we will abort connection
>>> anyway after switch {}) but
>>> it looks strange.
>>>
>>>> + client->structured_reply = true;
>>>> + myflags |= NBD_FLAG_SEND_DF;
>>>> + break;
>> Indeed, these two lines are harmless due to the catch-all 'ret < 0'
>> after the switch at the tail end of the loop (which is why I dropped the
>> 'if' here).
>>
>>
>
> yes. but it looks strange) Anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> On second thought, modifying client->structured_reply when we successfully replied with an error is wrong. So v6 will hoist the modifications into the last else branch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
