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

Ben Pfaff blp at ovn.org
Fri Apr 6 19:50:25 UTC 2018


On Fri, Apr 06, 2018 at 02:27:16PM -0500, Mark Michelson wrote:
> 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?

I see that I still didn't read the code carefully.

Let's get the code compiling with Justin's patches or an equivalent, and
then improve it from there.

Possibly a pipe isn't needed.  One possibility for passing data between
threads is to use a guarded_list from guarded-list.h along with a seq
from seq.h.


More information about the dev mailing list