[libcamera-devel] v4l2: v4l2_camera_proxy, v4l2_camera: Check return values of read/write

Message ID 20200616105434.57451-1-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] v4l2: v4l2_camera_proxy, v4l2_camera: Check return values of read/write
Related show

Commit Message

Paul Elder June 16, 2020, 10:54 a.m. UTC
The return value of the write to the eventfd (to signal POLLIN) from
V4L2Camera and the read from the eventfd (to clear POLLIN) from
V4L2CameraProxy was not ignored. Check the return value, and print an
error message.

Reported-by: Coverity CID=290743
Reported-by: Coverity CID=290744
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/v4l2/v4l2_camera.cpp       | 4 +++-
 src/v4l2/v4l2_camera_proxy.cpp | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

Comments

Umang Jain June 16, 2020, 2:34 p.m. UTC | #1
Hi Paul,

Thank you for the patch.

On 6/16/20 4:24 PM, Paul Elder wrote:
> The return value of the write to the eventfd (to signal POLLIN) from
> V4L2Camera and the read from the eventfd (to clear POLLIN) from
> V4L2CameraProxy was not ignored. Check the return value, and print an
Maybe you mean s/was not ignored/should not be ignored ?
> error message.
>
> Reported-by: Coverity CID=290743
> Reported-by: Coverity CID=290744
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>   src/v4l2/v4l2_camera.cpp       | 4 +++-
>   src/v4l2/v4l2_camera_proxy.cpp | 4 +++-
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 3c36932..9a1ebc8 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -91,7 +91,9 @@ void V4L2Camera::requestComplete(Request *request)
>   	bufferLock_.unlock();
>   
>   	uint64_t data = 1;
> -	::write(efd_, &data, sizeof(data));
> +	int ret = ::write(efd_, &data, sizeof(data));
> +	if (ret != sizeof(data))
> +		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
>   
>   	bufferSema_.release();
>   }
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 308a8ab..17477ab 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -454,7 +454,9 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>   	currentBuf_ = (currentBuf_ + 1) % bufferCount_;
>   
>   	uint64_t data;
> -	::read(efd_, &data, sizeof(data));
> +	ret = ::read(efd_, &data, sizeof(data));
> +	if (ret != sizeof(data))
> +		LOG(V4L2Compat, Error) << "Failed to clear eventfd POLLIN";
>   
>   	return 0;
>   }
Laurent Pinchart June 17, 2020, 12:31 a.m. UTC | #2
Hi Paul,

Thank you for the patch.

On Tue, Jun 16, 2020 at 07:54:34PM +0900, Paul Elder wrote:
> The return value of the write to the eventfd (to signal POLLIN) from
> V4L2Camera and the read from the eventfd (to clear POLLIN) from
> V4L2CameraProxy was not ignored. Check the return value, and print an

As pointed out by Umang, "was not ignored" doesn't seem right.

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> error message.
> 
> Reported-by: Coverity CID=290743
> Reported-by: Coverity CID=290744
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera.cpp       | 4 +++-
>  src/v4l2/v4l2_camera_proxy.cpp | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 3c36932..9a1ebc8 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -91,7 +91,9 @@ void V4L2Camera::requestComplete(Request *request)
>  	bufferLock_.unlock();
>  
>  	uint64_t data = 1;
> -	::write(efd_, &data, sizeof(data));
> +	int ret = ::write(efd_, &data, sizeof(data));
> +	if (ret != sizeof(data))
> +		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
>  
>  	bufferSema_.release();
>  }
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 308a8ab..17477ab 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -454,7 +454,9 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>  	currentBuf_ = (currentBuf_ + 1) % bufferCount_;
>  
>  	uint64_t data;
> -	::read(efd_, &data, sizeof(data));
> +	ret = ::read(efd_, &data, sizeof(data));
> +	if (ret != sizeof(data))
> +		LOG(V4L2Compat, Error) << "Failed to clear eventfd POLLIN";
>  
>  	return 0;
>  }

Patch

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 3c36932..9a1ebc8 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -91,7 +91,9 @@  void V4L2Camera::requestComplete(Request *request)
 	bufferLock_.unlock();
 
 	uint64_t data = 1;
-	::write(efd_, &data, sizeof(data));
+	int ret = ::write(efd_, &data, sizeof(data));
+	if (ret != sizeof(data))
+		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
 
 	bufferSema_.release();
 }
diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
index 308a8ab..17477ab 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -454,7 +454,9 @@  int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
 	currentBuf_ = (currentBuf_ + 1) % bufferCount_;
 
 	uint64_t data;
-	::read(efd_, &data, sizeof(data));
+	ret = ::read(efd_, &data, sizeof(data));
+	if (ret != sizeof(data))
+		LOG(V4L2Compat, Error) << "Failed to clear eventfd POLLIN";
 
 	return 0;
 }