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
