[libcamera-devel] [PATCH v2 6/7] v4l2: v4l2_compat: Add eventfd signaling to support polling
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jun 8 21:06:45 CEST 2020
Hi Paul,
This patch has introduced some warnings from coverity scan:
On 05/06/2020 10:01, Paul Elder wrote:
> To support polling, we need to be able to signal when data is
> available to be read (POLLIN), as well as events (POLLPRI). Add the
> necessary calls to eventfd to allow signaling POLLIN. We signal POLLIN
> by writing writing to the eventfd, and clear it by reading from the
> eventfd, upon VIDIOC_DQBUF.
>
> Note that eventfd does not support signaling POLLPRI, so we don't yet
> support V4L2 events.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2: use eventfd instead of socket
> ---
> src/v4l2/v4l2_camera.cpp | 9 +++++++++
> src/v4l2/v4l2_camera.h | 3 +++
> src/v4l2/v4l2_camera_proxy.cpp | 10 ++++++++++
> src/v4l2/v4l2_camera_proxy.h | 4 ++++
> src/v4l2/v4l2_compat_manager.cpp | 7 ++++++-
> 5 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 4da01a45..3c369328 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -8,6 +8,7 @@
> #include "v4l2_camera.h"
>
> #include <errno.h>
> +#include <unistd.h>
>
> #include "libcamera/internal/log.h"
>
> @@ -53,6 +54,11 @@ void V4L2Camera::close()
> camera_->release();
> }
>
> +void V4L2Camera::bind(int efd)
> +{
> + efd_ = efd;
> +}
> +
> void V4L2Camera::getStreamConfig(StreamConfiguration *streamConfig)
> {
> *streamConfig = config_->at(0);
> @@ -84,6 +90,9 @@ void V4L2Camera::requestComplete(Request *request)
> completedBuffers_.push_back(std::move(metadata));
> bufferLock_.unlock();
>
> + uint64_t data = 1;
> + ::write(efd_, &data, sizeof(data));
** CID 290743: Error handling issues (CHECKED_RETURN)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera.cpp:
94 in V4L2Camera::requestComplete(libcamera::Request *)()
________________________________________________________________________________________________________
*** CID 290743: Error handling issues (CHECKED_RETURN)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera.cpp:
94 in V4L2Camera::requestComplete(libcamera::Request *)()
88 std::unique_ptr<Buffer> metadata =
89 std::make_unique<Buffer>(request->cookie(), buffer->metadata());
90 completedBuffers_.push_back(std::move(metadata));
91 bufferLock_.unlock();
92
93 uint64_t data = 1;
>>> CID 290743: Error handling issues (CHECKED_RETURN)
>>> Calling "write" without checking return value (as is done
elsewhere 10 out of 12 times).
94 ::write(efd_, &data, sizeof(data));
95
96 bufferSema_.release();
97 }
98
99 int V4L2Camera::configure(StreamConfiguration *streamConfigOut,
If you choose to fix this, please add the tag
Reported-by: Coverity CID=290743
Or if you believe it is a false positive and should be ignored, let me
know and I'll update the CID in the system.
> +
> bufferSema_.release();
> }
>
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 303eda44..33f5eb02 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -39,6 +39,7 @@ public:
>
> int open();
> void close();
> + void bind(int efd);
> void getStreamConfig(StreamConfiguration *streamConfig);
> std::vector<Buffer> completedBuffers();
>
> @@ -70,6 +71,8 @@ private:
>
> std::deque<std::unique_ptr<Request>> pendingRequests_;
> std::deque<std::unique_ptr<Buffer>> completedBuffers_;
> +
> + int efd_;
** CID 290742: Uninitialized members (UNINIT_CTOR)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera.cpp:
23 in V4L2Camera::V4L2Camera(std::shared_ptr<libcamera::Camera>)()
________________________________________________________________________________________________________
*** CID 290742: Uninitialized members (UNINIT_CTOR)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera.cpp:
23 in V4L2Camera::V4L2Camera(std::shared_ptr<libcamera::Camera>)()
17 LOG_DECLARE_CATEGORY(V4L2Compat);
18
19 V4L2Camera::V4L2Camera(std::shared_ptr<Camera> camera)
20 : camera_(camera), isRunning_(false), bufferAllocator_(nullptr)
21 {
22 camera_->requestCompleted.connect(this,
&V4L2Camera::requestComplete);
>>> CID 290742: Uninitialized members (UNINIT_CTOR)
>>> Non-static class member "efd_" is not initialized in this
constructor nor in any functions that it calls.
23 }
24
25 V4L2Camera::~V4L2Camera()
26 {
27 close();
28 }
If you choose to fix this, please add the tag
Reported-by: Coverity CID=290742
Or if you believe it is a false positive and should be ignored, let me
know and I'll update the CID in the system.
> };
>
> #endif /* __V4L2_CAMERA_H__ */
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index cbe9e026..7ee4c0cb 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -13,6 +13,7 @@
> #include <linux/videodev2.h>
> #include <string.h>
> #include <sys/mman.h>
> +#include <unistd.h>
>
> #include <libcamera/camera.h>
> #include <libcamera/object.h>
> @@ -451,6 +452,9 @@ int V4L2CameraProxy::vidioc_dqbuf(struct v4l2_buffer *arg)
>
> currentBuf_ = (currentBuf_ + 1) % bufferCount_;
>
> + uint64_t data;
> + ::read(efd_, &data, sizeof(data));
> +
** CID 290744: Error handling issues (CHECKED_RETURN)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera_proxy.cpp:
456 in V4L2CameraProxy::vidioc_dqbuf(v4l2_buffer *)()
________________________________________________________________________________________________________
*** CID 290744: Error handling issues (CHECKED_RETURN)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/v4l2/v4l2_camera_proxy.cpp:
456 in V4L2CameraProxy::vidioc_dqbuf(v4l2_buffer *)()
450 buf.length = sizeimage_;
451 *arg = buf;
452
453 currentBuf_ = (currentBuf_ + 1) % bufferCount_;
454
455 uint64_t data;
>>> CID 290744: Error handling issues (CHECKED_RETURN)
>>> "read(int, void *, size_t)" returns the number of bytes read,
but it is ignored.
456 ::read(efd_, &data, sizeof(data));
457
458 return 0;
459 }
460
461 int V4L2CameraProxy::vidioc_streamon(int *arg)
If you choose to fix this, please add the tag
Reported-by: Coverity CID=290744
Or if you believe it is a false positive and should be ignored, let me
know and I'll update the CID in the system.
> return 0;
> }
>
> @@ -529,6 +533,12 @@ int V4L2CameraProxy::ioctl(unsigned long request, void *arg)
> return ret;
> }
>
> +void V4L2CameraProxy::bind(int fd)
> +{
> + efd_ = fd;
> + vcam_->bind(fd);
> +}
> +
> struct PixelFormatPlaneInfo {
> unsigned int bitsPerPixel;
> unsigned int hSubSampling;
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index af9f9bbe..7c65c886 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -33,6 +33,8 @@ public:
>
> int ioctl(unsigned long request, void *arg);
>
> + void bind(int fd);
> +
> private:
> bool validateBufferType(uint32_t type);
> bool validateMemoryType(uint32_t memory);
> @@ -77,6 +79,8 @@ private:
> std::map<void *, unsigned int> mmaps_;
>
> std::unique_ptr<V4L2Camera> vcam_;
> +
> + int efd_;
Or possibly this one for 290742, I'm not sure ;-)
> };
>
> #endif /* __V4L2_CAMERA_PROXY_H__ */
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index 2338a0ee..56533c4f 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -155,12 +155,17 @@ int V4L2CompatManager::openat(int dirfd, const char *path, int oflag, mode_t mod
> if (ret < 0)
> return ret;
>
> - int efd = eventfd(0, oflag & (O_CLOEXEC | O_NONBLOCK));
> + int efd = eventfd(0, EFD_SEMAPHORE |
> + ((oflag & O_CLOEXEC) ? EFD_CLOEXEC : 0) |
> + ((oflag & O_NONBLOCK) ? EFD_NONBLOCK : 0));
> if (efd < 0) {
> + int err = errno;
> proxy->close();
> + errno = err;
> return efd;
> }
>
> + proxy->bind(efd);
> devices_.emplace(efd, proxy);
>
> return efd;
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list