Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls

[Eric Blake]

On 02/10/2017 05:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> 09.02.2017 17:41, Eric Blake wrote:
>> On 02/09/2017 12:20 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 07.02.2017 19:32, Eric Blake wrote:
>>>> On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>>> Split out nbd_receive_simple_option to be reused for structured reply
>>>>> option.
>>>>> +    return "<unknown option>";
>>>> Can you please consider making this include the %d representation of
>>>> the
>>>> unknown option; perhaps by snprintf'ing into static storage?  While it
>>> Hmm.. The caller should free it in this case.
>> Only if you print it into malloc'd space. I think that printing it into
>> static storage may be sufficient (although then we have a race if more
>> than one thread is trying to use that static storage at the same time -
>> but do we ever have more than one thread trying to handle an error at
>> the same time?).
>>
> 
> This race would be if one thread decides to print two option names in
> one message. Or save one option in a var, then print other, then print var.

NBD_OPT_ are supposed to be handled sequentially. The NBD spec does NOT
allow for a single client to send a second NBD_OPT_ until after the
first one has received a final response.  So the only time we would have
two threads dealing with things in nbd/client.c during handshake is if
we have a single qemu process connecting as client to two different NBD
servers in parallel, where both servers issue an otherwise unknown opt
response at the same time, which I don't see as likely enough to be
worth avoiding static storage.

If you're still worried about the race (I'm not), to the point that you
don't want to use static storage just to avoid g_malloc(), then another
option is to make nbd_opt_name() take an input parameter for a buffer
and max size, and let the caller provide stack-based storage for
computing the resulting message (similar to how strerror_r does things).

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

signature.asc
Description: OpenPGP digital signature