[ovs-dev] [PATCH] stopwatch: Explicitly ignore write() return value.

Mark Michelson mmichels at redhat.com
Fri Apr 6 19:27:16 UTC 2018


On 04/06/2018 02:18 PM, Mark Michelson wrote:
> On 04/06/2018 12:28 PM, Ben Pfaff wrote:
>> On Fri, Apr 06, 2018 at 10:16:24AM -0700, Justin Pettit wrote:
>>> In some environments, builds would fail with the following error:
>>>
>>>    lib/stopwatch.c: In function ‘stopwatch_exit’:
>>>    lib/stopwatch.c:448:5: error: ignoring return value of ‘write’, 
>>> declared
>>>    with attribute warn_unused_result [-Werror=unused-result]
>>>        write(stopwatch_pipe[1], &pkt, sizeof pkt);
>>>
>>> This patch explicitly ignores the return value of write().
>>>
>>> This also fixes some minor coding style issues.
>>>
>>> Signed-off-by: Justin Pettit <jpettit at ovn.org>
>>
>> Obviously I didn't review this as carefully as I should have.  That's on
>> me.  I apologize.
>>
>> I believe that we already have a proper abstraction for what's going on
>> here: a latch.  The latch implementation also hides differences between
>> Unix and Windows (which this inline implementation isn't doing).  Would
>> you mind making this use latch.h instead of raw pipes?
>>
>> Would you mind breaking the style issues into a separate commit?
>>
>> Thanks,
>>
>> Ben.
> 
> Hi Ben,
> 
> Thanks for pointing out the latch library. I'll post the patch with this 
> change.
> 
> Mark!

I just had a look, and unfortunately, the current code doesn't translate 
directly to a latch. The reason is that you can't control the data that 
is sent to and read from the latch. It's essentially a boolean of "set" 
or "not set". In the current stopwatch implementation, data packets are 
written to the pipe and that data is then read by another thread. The 
actual data is important in this case.

You're 100% right that this needs to handle the Windows case. One idea I 
have is to create a cross-platform "pipe" library. Then the latch 
library and the stopwatch library can be built on top of the pipe 
library. What do you think about that?

Mark!


More information about the dev mailing list