[libcamera-devel] [PATCH 05/14] test: event-thread: Fix compilation on Chromium OS

[Laurent Pinchart]

Hi Jacopo,

On Mon, Aug 19, 2019 at 06:03:04PM +0200, Jacopo Mondi wrote:
> On Sun, Aug 18, 2019 at 08:26:36PM +0300, Laurent Pinchart wrote:
> > On Sun, Aug 18, 2019 at 07:24:58PM +0200, Jacopo Mondi wrote:
> >> On Sun, Aug 18, 2019 at 04:13:20AM +0300, Laurent Pinchart wrote:
> >>> Commit 92b4af98cd67 ("test: Add EventNotifier thread move test") causes
> >>> the build to fail in the Chromium OS build environment, because the
> >>> return values of the pipe() function marked with the
> >>> __warn_unused_result__ attribute is ignored. Fix this.
> >>>
> >>
> >> Do we want to compile tests on CrOS?
> >
> > I thought we didn't, and went investigating. Turns out there's a typo in
> > the ebuild, so tests are always compiled. I'll fix that. It's a good
> > test though :-)
> 
> And do we want to keep building tests on cros ?
> I would say why not, actually... It already spotted something

Yes, they're useful for this purpose :-)

> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>> ---
> >>>  test/event-thread.cpp | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/test/event-thread.cpp b/test/event-thread.cpp
> >>> index 714bc9845820..01120733eca4 100644
> >>> --- a/test/event-thread.cpp
> >>> +++ b/test/event-thread.cpp
> >>> @@ -25,7 +25,11 @@ public:
> >>>  	EventHandler()
> >>>  		: notified_(false)
> >>>  	{
> >>> -		pipe(pipefd_);
> >>> +		int ret = pipe(pipefd_);
> >>> +		if (ret < 0) {
> >>> +			ret = errno;
> >>> +			cout << "pipe() failed: " << strerror(ret) << endl;
> >>> +		}
> >>
> >> Shouldn't we fail the test if creating the pipe fails?
> >> This will require moving this out of the constructor probably...
> >
> > The test will eventually fail as the event will not be notified, so I
> > think we're good.
> 
> Thanks for the clarification.
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> >>>  		notifier_ = new EventNotifier(pipefd_[0], EventNotifier::Read, this);
> >>>  		notifier_->activated.connect(this, &EventHandler::readReady);

-- 
Regards,

Laurent Pinchart