[libvirt] [PATCHv2] util: ensure safe{read, write, zero} return is checked

[Chris Lalancette]

On 03/09/2010 02:09 PM, Eric Blake wrote:
>>>          remoteDriverLock(priv);
>>>
>>>          if (fds[1].revents) {
>>>              DEBUG0("Woken up from poll by other thread");
>>> -            saferead(priv->wakeupReadFD, &ignore, sizeof(ignore));
>>> +            ignore_value (saferead(priv->wakeupReadFD, &ignore,
>>> +                                   sizeof(ignore)));
>>
>> Hm.  Are we sure we want to ignore the return value here?  On the
>> one hand, the fact that we failed to read this byte is irrelevant;
>> it has done it's job to wake us out of poll.  On the other hand,
>> if we can't read this one byte, that probably means something more
>> serious is wrong and we might (probably will?) have problems later.
>> Should we goto error here?
> 
> Not the first instance of this; see, for example, daemon/event.c,
> virEventHandleWakeup().  If we change how wakeups are handled, it should
> be a separate patch, applied to all instances.

Yes and no.  Yes, virEventHandleWakeup() does have a similar problem.
However, the callback function is an exported prototype, so we can't change it
to have a return value now (and though we might be able to play tricks with
the opaque pointer, we can't guarantee all callback users will follow
the tricks).  Personally I would fix this one now and we could think about
the wakeup problem with virEventHandleWakeup() later, but I won't block
the patch for the (pretty minor) point.

-- 
Chris Lalancette