[libcamera-devel,v5,4/4] libcamera: v4l2: Standardize return value checks
diff mbox series

Message ID 20190619110548.20742-5-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • libcamera: Introduce V4L2Device base class
Related show

Commit Message

Jacopo Mondi June 19, 2019, 11:05 a.m. UTC 
Standardize all return values checks on the 'if (ret)' style where
appropriate.

No functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/v4l2_subdevice.cpp   |  2 +-
 src/libcamera/v4l2_videodevice.cpp | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Laurent Pinchart June 19, 2019, 12:30 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Jun 19, 2019 at 01:05:48PM +0200, Jacopo Mondi wrote:
> Standardize all return values checks on the 'if (ret)' style where
> appropriate.

Just curious, why did you choose (ret) over (ret < 0) ? I would have
gone the other way around :-)

> 
> No functional changes intended.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/v4l2_subdevice.cpp   |  2 +-
>  src/libcamera/v4l2_videodevice.cpp | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index d0e1d717b26c..7f887e377c97 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -340,7 +340,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>  	sel.r.height = rect->h;
>  
>  	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Unable to set rectangle " << target << " on pad "
>  			<< pad << ": " << strerror(-ret);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index bcbac79e82e9..d1409fcafa04 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -305,11 +305,11 @@ int V4L2VideoDevice::open()
>  	int ret;
>  
>  	ret = V4L2Device::open(O_RDWR | O_NONBLOCK);
> -	if (ret < 0)
> +	if (ret)
>  		return ret;
>  
>  	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to query device capabilities: "
>  			<< strerror(-ret);
> @@ -633,7 +633,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
>  	rb.memory = memoryType_;
>  
>  	ret = ioctl(VIDIOC_REQBUFS, &rb);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Unable to request " << count << " buffers: "
>  			<< strerror(-ret);
> @@ -684,7 +684,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>  		buf.m.planes = planes;
>  
>  		ret = ioctl(VIDIOC_QUERYBUF, &buf);
> -		if (ret < 0) {
> +		if (ret) {
>  			LOG(V4L2, Error)
>  				<< "Unable to query buffer " << i << ": "
>  				<< strerror(-ret);
> @@ -736,7 +736,7 @@ int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	expbuf.flags = O_RDWR;
>  
>  	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to export buffer: " << strerror(-ret);
>  		return ret;
> @@ -927,7 +927,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
>  	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
>  
>  	ret = ioctl(VIDIOC_QBUF, &buf);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to queue buffer " << buf.index << ": "
>  			<< strerror(-ret);
> @@ -963,7 +963,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
>  	}
>  
>  	ret = ioctl(VIDIOC_DQBUF, &buf);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to dequeue buffer: " << strerror(-ret);
>  		return nullptr;
> @@ -1022,7 +1022,7 @@ int V4L2VideoDevice::streamOn()
>  	int ret;
>  
>  	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to start streaming: " << strerror(-ret);
>  		return ret;
> @@ -1044,7 +1044,7 @@ int V4L2VideoDevice::streamOff()
>  	int ret;
>  
>  	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> -	if (ret < 0) {
> +	if (ret) {
>  		LOG(V4L2, Error)
>  			<< "Failed to stop streaming: " << strerror(-ret);
>  		return ret;
Jacopo Mondi June 19, 2019, 12:39 p.m. UTC | #2 
Hi Laurent,

On Wed, Jun 19, 2019 at 03:30:18PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Jun 19, 2019 at 01:05:48PM +0200, Jacopo Mondi wrote:
> > Standardize all return values checks on the 'if (ret)' style where
> > appropriate.
>
> Just curious, why did you choose (ret) over (ret < 0) ? I would have
> gone the other way around :-)
>

I would say tastes ? And it makes clear that the only valid result is
0, but that does not depend on the caller.

Should I drop the patch?

> >
> > No functional changes intended.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/v4l2_subdevice.cpp   |  2 +-
> >  src/libcamera/v4l2_videodevice.cpp | 18 +++++++++---------
> >  2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index d0e1d717b26c..7f887e377c97 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -340,7 +340,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> >  	sel.r.height = rect->h;
> >
> >  	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Unable to set rectangle " << target << " on pad "
> >  			<< pad << ": " << strerror(-ret);
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index bcbac79e82e9..d1409fcafa04 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -305,11 +305,11 @@ int V4L2VideoDevice::open()
> >  	int ret;
> >
> >  	ret = V4L2Device::open(O_RDWR | O_NONBLOCK);
> > -	if (ret < 0)
> > +	if (ret)
> >  		return ret;
> >
> >  	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to query device capabilities: "
> >  			<< strerror(-ret);
> > @@ -633,7 +633,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
> >  	rb.memory = memoryType_;
> >
> >  	ret = ioctl(VIDIOC_REQBUFS, &rb);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Unable to request " << count << " buffers: "
> >  			<< strerror(-ret);
> > @@ -684,7 +684,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
> >  		buf.m.planes = planes;
> >
> >  		ret = ioctl(VIDIOC_QUERYBUF, &buf);
> > -		if (ret < 0) {
> > +		if (ret) {
> >  			LOG(V4L2, Error)
> >  				<< "Unable to query buffer " << i << ": "
> >  				<< strerror(-ret);
> > @@ -736,7 +736,7 @@ int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
> >  	expbuf.flags = O_RDWR;
> >
> >  	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to export buffer: " << strerror(-ret);
> >  		return ret;
> > @@ -927,7 +927,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
> >  	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
> >
> >  	ret = ioctl(VIDIOC_QBUF, &buf);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to queue buffer " << buf.index << ": "
> >  			<< strerror(-ret);
> > @@ -963,7 +963,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> >  	}
> >
> >  	ret = ioctl(VIDIOC_DQBUF, &buf);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to dequeue buffer: " << strerror(-ret);
> >  		return nullptr;
> > @@ -1022,7 +1022,7 @@ int V4L2VideoDevice::streamOn()
> >  	int ret;
> >
> >  	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to start streaming: " << strerror(-ret);
> >  		return ret;
> > @@ -1044,7 +1044,7 @@ int V4L2VideoDevice::streamOff()
> >  	int ret;
> >
> >  	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> > -	if (ret < 0) {
> > +	if (ret) {
> >  		LOG(V4L2, Error)
> >  			<< "Failed to stop streaming: " << strerror(-ret);
> >  		return ret;
>
> --
> Regards,
>
> Laurent Pinchart
Laurent Pinchart June 19, 2019, 12:44 p.m. UTC | #3 
Hi Jacopo,

On Wed, Jun 19, 2019 at 02:39:56PM +0200, Jacopo Mondi wrote:
> On Wed, Jun 19, 2019 at 03:30:18PM +0300, Laurent Pinchart wrote:
> > On Wed, Jun 19, 2019 at 01:05:48PM +0200, Jacopo Mondi wrote:
> > > Standardize all return values checks on the 'if (ret)' style where
> > > appropriate.
> >
> > Just curious, why did you choose (ret) over (ret < 0) ? I would have
> > gone the other way around :-)
> >
> 
> I would say tastes ? And it makes clear that the only valid result is
> 0, but that does not depend on the caller.
> 
> Should I drop the patch?

I'm not opposed to the patch, but I'm wondering if we should make this
library-wide (in further patches possibly) and document it in our coding
style.

Note that a few ioctls return a non-negative value other than zero on
success. According to the man page errors are always indicated by -1.

> > >
> > > No functional changes intended.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  src/libcamera/v4l2_subdevice.cpp   |  2 +-
> > >  src/libcamera/v4l2_videodevice.cpp | 18 +++++++++---------
> > >  2 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > > index d0e1d717b26c..7f887e377c97 100644
> > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > @@ -340,7 +340,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > >  	sel.r.height = rect->h;
> > >
> > >  	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Unable to set rectangle " << target << " on pad "
> > >  			<< pad << ": " << strerror(-ret);
> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > > index bcbac79e82e9..d1409fcafa04 100644
> > > --- a/src/libcamera/v4l2_videodevice.cpp
> > > +++ b/src/libcamera/v4l2_videodevice.cpp
> > > @@ -305,11 +305,11 @@ int V4L2VideoDevice::open()
> > >  	int ret;
> > >
> > >  	ret = V4L2Device::open(O_RDWR | O_NONBLOCK);
> > > -	if (ret < 0)
> > > +	if (ret)
> > >  		return ret;
> > >
> > >  	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to query device capabilities: "
> > >  			<< strerror(-ret);
> > > @@ -633,7 +633,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
> > >  	rb.memory = memoryType_;
> > >
> > >  	ret = ioctl(VIDIOC_REQBUFS, &rb);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Unable to request " << count << " buffers: "
> > >  			<< strerror(-ret);
> > > @@ -684,7 +684,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
> > >  		buf.m.planes = planes;
> > >
> > >  		ret = ioctl(VIDIOC_QUERYBUF, &buf);
> > > -		if (ret < 0) {
> > > +		if (ret) {
> > >  			LOG(V4L2, Error)
> > >  				<< "Unable to query buffer " << i << ": "
> > >  				<< strerror(-ret);
> > > @@ -736,7 +736,7 @@ int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
> > >  	expbuf.flags = O_RDWR;
> > >
> > >  	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to export buffer: " << strerror(-ret);
> > >  		return ret;
> > > @@ -927,7 +927,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
> > >  	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
> > >
> > >  	ret = ioctl(VIDIOC_QBUF, &buf);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to queue buffer " << buf.index << ": "
> > >  			<< strerror(-ret);
> > > @@ -963,7 +963,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > >  	}
> > >
> > >  	ret = ioctl(VIDIOC_DQBUF, &buf);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to dequeue buffer: " << strerror(-ret);
> > >  		return nullptr;
> > > @@ -1022,7 +1022,7 @@ int V4L2VideoDevice::streamOn()
> > >  	int ret;
> > >
> > >  	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to start streaming: " << strerror(-ret);
> > >  		return ret;
> > > @@ -1044,7 +1044,7 @@ int V4L2VideoDevice::streamOff()
> > >  	int ret;
> > >
> > >  	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> > > -	if (ret < 0) {
> > > +	if (ret) {
> > >  		LOG(V4L2, Error)
> > >  			<< "Failed to stop streaming: " << strerror(-ret);
> > >  		return ret;
Jacopo Mondi June 19, 2019, 1:03 p.m. UTC | #4 
Hi Laurent,

On Wed, Jun 19, 2019 at 03:44:54PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, Jun 19, 2019 at 02:39:56PM +0200, Jacopo Mondi wrote:
> > On Wed, Jun 19, 2019 at 03:30:18PM +0300, Laurent Pinchart wrote:
> > > On Wed, Jun 19, 2019 at 01:05:48PM +0200, Jacopo Mondi wrote:
> > > > Standardize all return values checks on the 'if (ret)' style where
> > > > appropriate.
> > >
> > > Just curious, why did you choose (ret) over (ret < 0) ? I would have
> > > gone the other way around :-)
> > >
> >
> > I would say tastes ? And it makes clear that the only valid result is
> > 0, but that does not depend on the caller.
> >
> > Should I drop the patch?
>
> I'm not opposed to the patch, but I'm wondering if we should make this
> library-wide (in further patches possibly) and document it in our coding
> style.
>
> Note that a few ioctls return a non-negative value other than zero on
> success. According to the man page errors are always indicated by -1.
>

For the ioctl system call that's indeed true, but here we're calling
the base class V4L2Device::ioctl() operation, which returns 0 on
success.

Anyway, no need to spend too much time on this. I'll drop the patch.

Thanks
  j
> > > >
> > > > No functional changes intended.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > > ---
> > > >  src/libcamera/v4l2_subdevice.cpp   |  2 +-
> > > >  src/libcamera/v4l2_videodevice.cpp | 18 +++++++++---------
> > > >  2 files changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > > > index d0e1d717b26c..7f887e377c97 100644
> > > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > > @@ -340,7 +340,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > > >  	sel.r.height = rect->h;
> > > >
> > > >  	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Unable to set rectangle " << target << " on pad "
> > > >  			<< pad << ": " << strerror(-ret);
> > > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > > > index bcbac79e82e9..d1409fcafa04 100644
> > > > --- a/src/libcamera/v4l2_videodevice.cpp
> > > > +++ b/src/libcamera/v4l2_videodevice.cpp
> > > > @@ -305,11 +305,11 @@ int V4L2VideoDevice::open()
> > > >  	int ret;
> > > >
> > > >  	ret = V4L2Device::open(O_RDWR | O_NONBLOCK);
> > > > -	if (ret < 0)
> > > > +	if (ret)
> > > >  		return ret;
> > > >
> > > >  	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to query device capabilities: "
> > > >  			<< strerror(-ret);
> > > > @@ -633,7 +633,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
> > > >  	rb.memory = memoryType_;
> > > >
> > > >  	ret = ioctl(VIDIOC_REQBUFS, &rb);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Unable to request " << count << " buffers: "
> > > >  			<< strerror(-ret);
> > > > @@ -684,7 +684,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
> > > >  		buf.m.planes = planes;
> > > >
> > > >  		ret = ioctl(VIDIOC_QUERYBUF, &buf);
> > > > -		if (ret < 0) {
> > > > +		if (ret) {
> > > >  			LOG(V4L2, Error)
> > > >  				<< "Unable to query buffer " << i << ": "
> > > >  				<< strerror(-ret);
> > > > @@ -736,7 +736,7 @@ int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
> > > >  	expbuf.flags = O_RDWR;
> > > >
> > > >  	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to export buffer: " << strerror(-ret);
> > > >  		return ret;
> > > > @@ -927,7 +927,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
> > > >  	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
> > > >
> > > >  	ret = ioctl(VIDIOC_QBUF, &buf);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to queue buffer " << buf.index << ": "
> > > >  			<< strerror(-ret);
> > > > @@ -963,7 +963,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
> > > >  	}
> > > >
> > > >  	ret = ioctl(VIDIOC_DQBUF, &buf);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to dequeue buffer: " << strerror(-ret);
> > > >  		return nullptr;
> > > > @@ -1022,7 +1022,7 @@ int V4L2VideoDevice::streamOn()
> > > >  	int ret;
> > > >
> > > >  	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to start streaming: " << strerror(-ret);
> > > >  		return ret;
> > > > @@ -1044,7 +1044,7 @@ int V4L2VideoDevice::streamOff()
> > > >  	int ret;
> > > >
> > > >  	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
> > > > -	if (ret < 0) {
> > > > +	if (ret) {
> > > >  		LOG(V4L2, Error)
> > > >  			<< "Failed to stop streaming: " << strerror(-ret);
> > > >  		return ret;
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index d0e1d717b26c..7f887e377c97 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -340,7 +340,7 @@  int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
 	sel.r.height = rect->h;
 
 	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Unable to set rectangle " << target << " on pad "
 			<< pad << ": " << strerror(-ret);
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index bcbac79e82e9..d1409fcafa04 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -305,11 +305,11 @@  int V4L2VideoDevice::open()
 	int ret;
 
 	ret = V4L2Device::open(O_RDWR | O_NONBLOCK);
-	if (ret < 0)
+	if (ret)
 		return ret;
 
 	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to query device capabilities: "
 			<< strerror(-ret);
@@ -633,7 +633,7 @@  int V4L2VideoDevice::requestBuffers(unsigned int count)
 	rb.memory = memoryType_;
 
 	ret = ioctl(VIDIOC_REQBUFS, &rb);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Unable to request " << count << " buffers: "
 			<< strerror(-ret);
@@ -684,7 +684,7 @@  int V4L2VideoDevice::exportBuffers(BufferPool *pool)
 		buf.m.planes = planes;
 
 		ret = ioctl(VIDIOC_QUERYBUF, &buf);
-		if (ret < 0) {
+		if (ret) {
 			LOG(V4L2, Error)
 				<< "Unable to query buffer " << i << ": "
 				<< strerror(-ret);
@@ -736,7 +736,7 @@  int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
 	expbuf.flags = O_RDWR;
 
 	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to export buffer: " << strerror(-ret);
 		return ret;
@@ -927,7 +927,7 @@  int V4L2VideoDevice::queueBuffer(Buffer *buffer)
 	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
 
 	ret = ioctl(VIDIOC_QBUF, &buf);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to queue buffer " << buf.index << ": "
 			<< strerror(-ret);
@@ -963,7 +963,7 @@  Buffer *V4L2VideoDevice::dequeueBuffer()
 	}
 
 	ret = ioctl(VIDIOC_DQBUF, &buf);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to dequeue buffer: " << strerror(-ret);
 		return nullptr;
@@ -1022,7 +1022,7 @@  int V4L2VideoDevice::streamOn()
 	int ret;
 
 	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to start streaming: " << strerror(-ret);
 		return ret;
@@ -1044,7 +1044,7 @@  int V4L2VideoDevice::streamOff()
 	int ret;
 
 	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
-	if (ret < 0) {
+	if (ret) {
 		LOG(V4L2, Error)
 			<< "Failed to stop streaming: " << strerror(-ret);
 		return ret;