From patchwork Sun Nov 28 23:57:36 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14816 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5484BBF415 for ; Sun, 28 Nov 2021 23:58:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D887260597; Mon, 29 Nov 2021 00:58:24 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ju6Z3zh9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BDD246011D for ; Mon, 29 Nov 2021 00:58:21 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 514E0A15; Mon, 29 Nov 2021 00:58:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143901; bh=k4enwmxaxoSZEZTkvEjJc1YiT75NXRItHbYIa/DDpgI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ju6Z3zh94BNeGChiXPF8mtFjcqD9+v0OTYL2XMxP/EiO4IAJ8e1ztrpKCgGtCmV6e FMDFQzpDd9jS3ss03odX9gEZGeldkFmQoJxGAtNqT4zCsziiSa7/1kp5+JFtd3Lz4d 0pRKLbGK7I+GvWzMuGFhfS55i+Zo6g724U/lWxDo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:36 +0200 Message-Id: <20211128235752.10836-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 01/17] libcamera: Move compiler.h to base/ X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In preparation for usage of __nodiscard in the base API, move the compiler.h header to base. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- include/libcamera/{ => base}/compiler.h | 0 include/libcamera/base/meson.build | 1 + include/libcamera/geometry.h | 2 +- include/libcamera/meson.build | 1 - 4 files changed, 2 insertions(+), 2 deletions(-) rename include/libcamera/{ => base}/compiler.h (100%) diff --git a/include/libcamera/compiler.h b/include/libcamera/base/compiler.h similarity index 100% rename from include/libcamera/compiler.h rename to include/libcamera/base/compiler.h diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index 525aba9d2919..23dd4e2023a8 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -6,6 +6,7 @@ libcamera_base_headers = files([ 'backtrace.h', 'bound_method.h', 'class.h', + 'compiler.h', 'event_dispatcher.h', 'event_dispatcher_poll.h', 'event_notifier.h', diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h index 713f71c180c6..7838b6793617 100644 --- a/include/libcamera/geometry.h +++ b/include/libcamera/geometry.h @@ -10,7 +10,7 @@ #include #include -#include +#include namespace libcamera { diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 7155ff203f6e..a8cca2a88cad 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -5,7 +5,6 @@ libcamera_include_dir = 'libcamera' / 'libcamera' libcamera_public_headers = files([ 'camera.h', 'camera_manager.h', - 'compiler.h', 'controls.h', 'file_descriptor.h', 'framebuffer.h', From patchwork Sun Nov 28 23:57:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14817 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 43552BF415 for ; Sun, 28 Nov 2021 23:58:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F2E78605B9; Mon, 29 Nov 2021 00:58:27 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="svYXkQ85"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 34A6D6011D for ; Mon, 29 Nov 2021 00:58:22 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BB29DE78; Mon, 29 Nov 2021 00:58:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143902; bh=ICFnIrs6nii9YubgKFNFUw3rd4VYIG9kXwIGhth5FwE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=svYXkQ85fvQs9u8rJ4IY2yD4YZIpgWquFvjlIa8I6OZzZwDzoirl49WZ46iaBbaam OybHb2xQv/kZYvgS26aoaqHs/Ok4qnrjMOhedLRgz7TAzLnPUE57MKPD1R9eh2q++K lphPEvtjUN+56++cu6o4+1h3/deE/sDjkAO03NBg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:37 +0200 Message-Id: <20211128235752.10836-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 02/17] libcamera: Move file_descriptor.h to base/ X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The FileDescriptor class is a generic helper that matches the criteria for the base library. Move it there. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- include/libcamera/{ => base}/file_descriptor.h | 0 include/libcamera/base/meson.build | 1 + include/libcamera/framebuffer.h | 3 +-- include/libcamera/internal/ipc_pipe.h | 3 +-- include/libcamera/meson.build | 1 - src/ipa/raspberrypi/raspberrypi.cpp | 2 +- src/libcamera/{ => base}/file_descriptor.cpp | 4 ++-- src/libcamera/base/meson.build | 1 + src/libcamera/meson.build | 1 - src/libcamera/pipeline/raspberrypi/dma_heaps.h | 2 +- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ++- src/libcamera/v4l2_videodevice.cpp | 3 +-- src/v4l2/v4l2_camera.h | 2 +- test/file-descriptor.cpp | 3 +-- 14 files changed, 13 insertions(+), 16 deletions(-) rename include/libcamera/{ => base}/file_descriptor.h (100%) rename src/libcamera/{ => base}/file_descriptor.cpp (99%) diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/base/file_descriptor.h similarity index 100% rename from include/libcamera/file_descriptor.h rename to include/libcamera/base/file_descriptor.h diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index 23dd4e2023a8..f73b00917409 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -11,6 +11,7 @@ libcamera_base_headers = files([ 'event_dispatcher_poll.h', 'event_notifier.h', 'file.h', + 'file_descriptor.h', 'flags.h', 'log.h', 'message.h', diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index 4e710e332370..2fbea9c5be16 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -13,10 +13,9 @@ #include #include +#include #include -#include - namespace libcamera { class Request; diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h index bd8824f255c2..986f8d886fa6 100644 --- a/include/libcamera/internal/ipc_pipe.h +++ b/include/libcamera/internal/ipc_pipe.h @@ -9,10 +9,9 @@ #include +#include #include -#include - #include "libcamera/internal/ipc_unixsocket.h" namespace libcamera { diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index a8cca2a88cad..5f42977c034b 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -6,7 +6,6 @@ libcamera_public_headers = files([ 'camera.h', 'camera_manager.h', 'controls.h', - 'file_descriptor.h', 'framebuffer.h', 'framebuffer_allocator.h', 'geometry.h', diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index fed82e223d38..c6aec09046f7 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -15,12 +15,12 @@ #include +#include #include #include #include #include -#include #include #include #include diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp similarity index 99% rename from src/libcamera/file_descriptor.cpp rename to src/libcamera/base/file_descriptor.cpp index 0409c3e1758c..f5f87c56eee8 100644 --- a/src/libcamera/file_descriptor.cpp +++ b/src/libcamera/base/file_descriptor.cpp @@ -5,7 +5,7 @@ * file_descriptor.cpp - File descriptor wrapper */ -#include +#include #include #include @@ -16,7 +16,7 @@ #include /** - * \file file_descriptor.h + * \file base/file_descriptor.h * \brief File descriptor wrapper */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index 05fed7acf561..d5254fda9cbf 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -8,6 +8,7 @@ libcamera_base_sources = files([ 'event_dispatcher_poll.cpp', 'event_notifier.cpp', 'file.cpp', + 'file_descriptor.cpp', 'flags.cpp', 'log.cpp', 'message.cpp', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 6727a777d804..626dfca176a4 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -14,7 +14,6 @@ libcamera_sources = files([ 'delayed_controls.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', - 'file_descriptor.cpp', 'formats.cpp', 'framebuffer.cpp', 'framebuffer_allocator.cpp', diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/raspberrypi/dma_heaps.h index 38dfc242c0b9..57beaeb2e48a 100644 --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.h +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.h @@ -7,7 +7,7 @@ #pragma once -#include +#include namespace libcamera { diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ad526a8be6a2..7526edf774a2 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -12,9 +12,10 @@ #include #include +#include + #include #include -#include #include #include #include diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 4f04212df672..0a85bcf6b3ff 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -22,11 +22,10 @@ #include #include +#include #include #include -#include - #include "libcamera/internal/formats.h" #include "libcamera/internal/framebuffer.h" #include "libcamera/internal/media_device.h" diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 94263b2f5db6..0cea111561dd 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -11,10 +11,10 @@ #include #include +#include #include #include -#include #include #include diff --git a/test/file-descriptor.cpp b/test/file-descriptor.cpp index 85b077a25c32..76badc4c5fad 100644 --- a/test/file-descriptor.cpp +++ b/test/file-descriptor.cpp @@ -11,8 +11,7 @@ #include #include -#include - +#include #include #include "test.h" From patchwork Sun Nov 28 23:57:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14818 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 03C78C324F for ; Sun, 28 Nov 2021 23:58:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4EDE060596; Mon, 29 Nov 2021 00:58:28 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hTso7Poh"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B312060591 for ; Mon, 29 Nov 2021 00:58:22 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 33965730; Mon, 29 Nov 2021 00:58:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143902; bh=vouL4op4AAtLX9vNiPG1Fp1rivj6Kz38JJ/WSGtPvWE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hTso7PohVggGe+nGoOZoLQwUiWOSCPVZdFDNvLqwcKccDwVEne9Z1j7c6EOA1Erli uFcCrQnSnY3YYGcgZWGmSgipIJh+lHtWCyQyZ96fzP8S1eSTFQ8vCaXjU7NimB3YLr eNOVK4tlUnkUkDBw/MUhOp0fkfEKf3MmYnKPtDG4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:38 +0200 Message-Id: <20211128235752.10836-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 03/17] libcamera: base: file_descriptor: Move inode() function to File class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The inode() function has always been a bit of an outcast in the FileDescriptor class, as it's not related to the core feature provided by FileDescriptor, a shared ownership wrapper around file descriptors. Move it to the File class instead. It may not be a perfect fit there either, but File is a private class, so the inode() function is at least not part of the public API anymore. Signed-off-by: Laurent Pinchart --- include/libcamera/base/file.h | 3 +++ include/libcamera/base/file_descriptor.h | 3 --- src/libcamera/base/file.cpp | 23 ++++++++++++++++++++++ src/libcamera/base/file_descriptor.cpp | 25 ------------------------ src/libcamera/framebuffer.cpp | 5 +++-- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h index 55e8edd934d4..996751a7ab72 100644 --- a/include/libcamera/base/file.h +++ b/include/libcamera/base/file.h @@ -20,6 +20,8 @@ namespace libcamera { +class FileDescriptor; + class File { public: @@ -66,6 +68,7 @@ public: bool unmap(uint8_t *addr); static bool exists(const std::string &name); + static ino_t inode(const FileDescriptor &fd); private: LIBCAMERA_DISABLE_COPY(File) diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h index 8d764f8b4a26..5d1594e80801 100644 --- a/include/libcamera/base/file_descriptor.h +++ b/include/libcamera/base/file_descriptor.h @@ -8,7 +8,6 @@ #pragma once #include -#include namespace libcamera { @@ -28,8 +27,6 @@ public: int fd() const { return fd_ ? fd_->fd() : -1; } FileDescriptor dup() const; - ino_t inode() const; - private: class Descriptor { diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp index ae4be1f95fa6..7043f9461cf7 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -14,6 +14,7 @@ #include #include +#include #include /** @@ -472,4 +473,26 @@ bool File::exists(const std::string &name) return !S_ISDIR(st.st_mode); } +/** + * \brief Retrieve the inode of a FileDescriptor + * + * \return The file descriptor inode on success, or 0 on error + */ +ino_t File::inode(const FileDescriptor &fd) +{ + if (!fd.isValid()) + return 0; + + struct stat st; + int ret = fstat(fd.fd(), &st); + if (ret < 0) { + ret = -errno; + LOG(File, Fatal) + << "Failed to fstat() fd: " << strerror(-ret); + return 0; + } + + return st.st_ino; +} + } /* namespace libcamera */ diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp index f5f87c56eee8..98d4b4bfd24f 100644 --- a/src/libcamera/base/file_descriptor.cpp +++ b/src/libcamera/base/file_descriptor.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -223,30 +222,6 @@ FileDescriptor FileDescriptor::dup() const return FileDescriptor(fd()); } -/** - * \brief Retrieve the file descriptor inode - * - * \todo Should this move to the File class ? - * - * \return The file descriptor inode on success, or 0 on error - */ -ino_t FileDescriptor::inode() const -{ - if (!isValid()) - return 0; - - struct stat st; - int ret = fstat(fd_->fd(), &st); - if (ret < 0) { - ret = -errno; - LOG(FileDescriptor, Fatal) - << "Failed to fstat() fd: " << strerror(-ret); - return 0; - } - - return st.st_ino; -} - FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate) { if (!duplicate) { diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index 337ea1155a38..f5bcf107d7aa 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -8,6 +8,7 @@ #include #include "libcamera/internal/framebuffer.h" +#include #include /** @@ -236,8 +237,8 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) */ if (plane.fd.fd() != planes_[0].fd.fd()) { if (!inode) - inode = planes_[0].fd.inode(); - if (plane.fd.inode() != inode) { + inode = File::inode(planes_[0].fd); + if (File::inode(plane.fd) != inode) { isContiguous = false; break; } From patchwork Sun Nov 28 23:57:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14819 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2A362BF415 for ; Sun, 28 Nov 2021 23:58:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BD17F605A4; Mon, 29 Nov 2021 00:58:28 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vjzY+uSW"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BE6F6058C for ; Mon, 29 Nov 2021 00:58:23 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A0623A15; Mon, 29 Nov 2021 00:58:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143902; bh=8FU//GerFGy0XOrloVcixQN3cv9UfE5ViIjIgSSbUrU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vjzY+uSWRIa/UOS1XhTkUNkpIg313Dc+en5E7GLpT1wmaSYk+s8Dtd97ZmwGF5UrV m2DZdW8g+E/kdFEz1+FN7HGgW4A7mQpP28gTkNBUtYr+FNkYb3drbd9FsA38uyw7n0 DATWjOAAgN7CzNuwP66Oud8VlruGK8zLqBdnpbgo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:39 +0200 Message-Id: <20211128235752.10836-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 04/17] libcamera: base: Introduce UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda This introduces UniqueFD. It acts like unique_ptr to a file descriptor. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- Changes since v2: - Rename ScopedFD to UniqueFD - Inline most functions to allow compiler optimizations - Bring the API closer to unique_ptr<> - Add swap() - Documentation cleanups - Slip FileDescriptor constructor to separate patch - Fix isValid() --- include/libcamera/base/meson.build | 1 + include/libcamera/base/unique_fd.h | 69 ++++++++++++++++ src/libcamera/base/meson.build | 1 + src/libcamera/base/unique_fd.cpp | 123 +++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+) create mode 100644 include/libcamera/base/unique_fd.h create mode 100644 src/libcamera/base/unique_fd.cpp diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index f73b00917409..cca374a769cc 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -22,6 +22,7 @@ libcamera_base_headers = files([ 'span.h', 'thread.h', 'timer.h', + 'unique_fd.h', 'utils.h', ]) diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h new file mode 100644 index 000000000000..ae4d96b75797 --- /dev/null +++ b/include/libcamera/base/unique_fd.h @@ -0,0 +1,69 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * unique_fd.h - File descriptor wrapper that owns a file descriptor. + */ + +#pragma once + +#include + +#include +#include + +namespace libcamera { + +class UniqueFD final +{ +public: + UniqueFD() + : fd_(-1) + { + } + + explicit UniqueFD(int fd) + : fd_(fd) + { + } + + UniqueFD(UniqueFD &&other) + : fd_(other.release()) + { + } + + ~UniqueFD() + { + reset(); + } + + UniqueFD &operator=(UniqueFD &&other) + { + reset(other.release()); + return *this; + } + + __nodiscard int release() + { + int fd = fd_; + fd_ = -1; + return fd; + } + + void reset(int fd = -1); + + void swap(UniqueFD &other) + { + std::swap(fd_, other.fd_); + } + + int get() const { return fd_; } + bool isValid() const { return fd_ >= 0; } + +private: + LIBCAMERA_DISABLE_COPY(UniqueFD) + + int fd_; +}; + +} /* namespace libcamera */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index d5254fda9cbf..b0d85bc19245 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -17,6 +17,7 @@ libcamera_base_sources = files([ 'signal.cpp', 'thread.cpp', 'timer.cpp', + 'unique_fd.cpp', 'utils.cpp', ]) diff --git a/src/libcamera/base/unique_fd.cpp b/src/libcamera/base/unique_fd.cpp new file mode 100644 index 000000000000..83d6919cf623 --- /dev/null +++ b/src/libcamera/base/unique_fd.cpp @@ -0,0 +1,123 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * unique_fd.cpp - File descriptor wrapper that owns a file descriptor + */ + +#include + +#include +#include + +#include + +/** + * \file base/unique_fd.h + * \brief File descriptor wrapper that owns a file descriptor + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(UniqueFD) + +/** + * \class UniqueFD + * \brief unique_ptr-like wrapper for a file descriptor + * + * The UniqueFD is a wrapper that owns and manages the lifetime of a file + * descriptor. It is constructed from a numerical file descriptor, and takes + * over its ownership. The file descriptor is closed when the UniqueFD is + * destroyed, or when it is assigned another file descriptor with operator=() + * or reset(). + */ + +/** + * \fn UniqueFD::UniqueFD() + * \brief Construct a UniqueFD that owns no file descriptor + */ + +/** + * \fn UniqueFD::UniqueFD(int fd) + * \brief Construct a UniqueFD that owns \a fd + * \param[in] fd A file descriptor to manage + */ + +/** + * \fn UniqueFD::UniqueFD(UniqueFD &&other) + * \brief Move constructor, create a UniqueFD by taking over \a other + * \param[in] other The other UniqueFD + * + * Create a UniqueFD by transferring ownership of the file descriptor owned by + * \a other. Upon return, the \a other UniqueFD is invalid. + */ + +/** + * \fn UniqueFD::~UniqueFD() + * \brief Destroy the UniqueFD instance + * + * If a file descriptor is owned, it is closed. + */ + +/** + * \fn UniqueFD::operator=(UniqueFD &&other) + * \brief Move assignment operator, replace a UniqueFD by taking over \a other + * \param[in] other The other UniqueFD + * + * If this UniqueFD owns a file descriptor, the file descriptor is closed + * first. The file descriptor is then replaced by the one of \a other. Upon + * return, \a other is invalid. + * + * \return A reference to this UniqueFD + */ + +/** + * \fn UniqueFD::release() + * \brief Release ownership of the file descriptor without closing it + * + * This function releases and returns the owned file descriptor without closing + * it. The caller owns the returned value and must take care of handling its + * life time to avoid file descriptor leakages. Upon return this UniqueFD is + * invalid. + * + * \return The managed file descriptor, or -1 if no file descriptor was owned + */ + +/** + * \brief Replace the managed file descriptor + * \param[in] fd The new file descriptor to manage + * + * Close the managed file descriptor, if any, and replace it with the new \a fd. + * + * Self-resetting (passing an \a fd already managed by this instance) is invalid + * and results in undefined behaviour. + */ +void UniqueFD::reset(int fd) +{ + ASSERT(!isValid() || fd != fd_); + + std::swap(fd, fd_); + + if (fd >= 0) + close(fd); +} + +/** + * \fn UniqueFD::swap(UniqueFD &other) + * \brief Swap the managed file descriptors with another UniqueFD + * \param[in] other Another UniqueFD to swap the file descriptor with + */ + +/** + * \fn UniqueFD::get() + * \brief Retrieve the managed file descriptor + * \return The managed file descriptor, or -1 if no file descriptor is owned + */ + +/** + * \fn UniqueFD::isValid() + * \brief Check if the UniqueFD owns a valid file descriptor + * \return True if the UniqueFD owns a valid file descriptor, false otherwise + */ + +} /* namespace libcamera */ From patchwork Sun Nov 28 23:57:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14820 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D356FC3250 for ; Sun, 28 Nov 2021 23:58:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4EE18605C7; Mon, 29 Nov 2021 00:58:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LwmooM1c"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 857156058D for ; Mon, 29 Nov 2021 00:58:23 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A229730; Mon, 29 Nov 2021 00:58:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143903; bh=X2908uUKV+7UXKdLPhFFZ16yLAjg3pFYBQLfoyokNTY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LwmooM1cQDdSxSTD7RXRXzKYaAuUSSpqNRjtpGJgLQz//vI1yiRgAVbq9tIbAzwcb uzvrL5Mys/WUSbjP1icbap2un6heIWlw2ut7VB/B2ghzbWbFK7SGyaRE8O5DD8mAHs jpJHY/DuBLNSWon0lAUYIMJ412FkgLN/oLt8rncM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:40 +0200 Message-Id: <20211128235752.10836-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 05/17] test: Add UniqueFD test X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a unit test to exercise the API of the UniqueFD class. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- test/meson.build | 1 + test/unique-fd.cpp | 220 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 test/unique-fd.cpp diff --git a/test/meson.build b/test/meson.build index d0466f17d7b6..42dfbc1f8ee9 100644 --- a/test/meson.build +++ b/test/meson.build @@ -53,6 +53,7 @@ internal_tests = [ ['threads', 'threads.cpp'], ['timer', 'timer.cpp'], ['timer-thread', 'timer-thread.cpp'], + ['unique-fd', 'unique-fd.cpp'], ['utils', 'utils.cpp'], ] diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp new file mode 100644 index 000000000000..92ca3f328811 --- /dev/null +++ b/test/unique-fd.cpp @@ -0,0 +1,220 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * unique-fd.cpp - UniqueFD test + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include "test.h" + +using namespace libcamera; +using namespace std; + +class UniqueFDTest : public Test +{ +protected: + int init() + { + return createFd(); + } + + int createFd() + { + fd_ = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR); + if (fd_ < 0) + return TestFail; + + /* Cache inode number of temp file. */ + struct stat s; + if (fstat(fd_, &s)) + return TestFail; + + inodeNr_ = s.st_ino; + + return 0; + } + + int run() + { + /* Test creating empty UniqueFD. */ + UniqueFD fd; + + if (fd.isValid() || fd.get() != -1) { + std::cout << "Failed fd check (default constructor)" + << std::endl; + return TestFail; + } + + /* Test creating UniqueFD from numerical file descriptor. */ + UniqueFD fd2(fd_); + if (!fd2.isValid() || fd2.get() != fd_) { + std::cout << "Failed fd check (fd constructor)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (fd constructor)" + << std::endl; + return TestFail; + } + + /* Test move constructor. */ + UniqueFD fd3(std::move(fd2)); + if (!fd3.isValid() || fd3.get() != fd_) { + std::cout << "Failed fd check (move constructor)" + << std::endl; + return TestFail; + } + + if (fd2.isValid() || fd2.get() != -1) { + std::cout << "Failed moved fd check (move constructor)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (move constructor)" + << std::endl; + return TestFail; + } + + /* Test move assignment operator. */ + fd = std::move(fd3); + if (!fd.isValid() || fd.get() != fd_) { + std::cout << "Failed fd check (move assignment)" + << std::endl; + return TestFail; + } + + if (fd3.isValid() || fd3.get() != -1) { + std::cout << "Failed moved fd check (move assignment)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (move assignment)" + << std::endl; + return TestFail; + } + + /* Test swapping. */ + fd2.swap(fd); + if (!fd2.isValid() || fd2.get() != fd_) { + std::cout << "Failed fd check (swap)" + << std::endl; + return TestFail; + } + + if (fd.isValid() || fd.get() != -1) { + std::cout << "Failed swapped fd check (swap)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (swap)" + << std::endl; + return TestFail; + } + + /* Test release. */ + int numFd = fd2.release(); + if (fd2.isValid() || fd2.get() != -1) { + std::cout << "Failed fd check (release)" + << std::endl; + return TestFail; + } + + if (numFd != fd_) { + std::cout << "Failed released fd check (release)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (release)" + << std::endl; + return TestFail; + } + + /* Test reset assignment. */ + fd.reset(numFd); + if (!fd.isValid() || fd.get() != fd_) { + std::cout << "Failed fd check (reset assignment)" + << std::endl; + return TestFail; + } + + if (!isValidFd(fd_)) { + std::cout << "Failed fd validity (reset assignment)" + << std::endl; + return TestFail; + } + + /* Test reset destruction. */ + fd.reset(); + if (fd.isValid() || fd.get() != -1) { + std::cout << "Failed fd check (reset destruction)" + << std::endl; + return TestFail; + } + + if (isValidFd(fd_)) { + std::cout << "Failed fd validity (reset destruction)" + << std::endl; + return TestFail; + } + + /* Test destruction. */ + if (createFd() == TestFail) { + std::cout << "Failed to recreate test fd" + << std::endl; + return TestFail; + } + + { + UniqueFD fd4(fd_); + } + + if (isValidFd(fd_)) { + std::cout << "Failed fd validity (destruction)" + << std::endl; + return TestFail; + } + + return TestPass; + } + + void cleanup() + { + if (fd_ > 0) + close(fd_); + } + +private: + bool isValidFd(int fd) + { + struct stat s; + if (fstat(fd, &s)) + return false; + + /* Check that inode number matches cached temp file. */ + return s.st_ino == inodeNr_; + } + + int fd_; + ino_t inodeNr_; +}; + +TEST_REGISTER(UniqueFDTest) From patchwork Sun Nov 28 23:57:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14821 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 810C2C324F for ; Sun, 28 Nov 2021 23:58:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 46C63605A5; Mon, 29 Nov 2021 00:58:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="bkyKTJqr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EA3CE6057D for ; Mon, 29 Nov 2021 00:58:23 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8388BA15; Mon, 29 Nov 2021 00:58:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143903; bh=DunH+O3Etm9haLA0VlTo9xZbonfp9MEcsbPRg1t21NY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bkyKTJqr194sYU+NWjCB1ghK5Gv3e+kgNF6sxabNPeucBZVks28WwI+x8GpRiKmgh pAnhK6lNu9rMmhKTjVuDqmoYiGCHJaLXQ1IAGQuSxtpBaxqFCnkMKZiY6KI2WclsR0 /yAoDXkANoCwT/B0zIJreIWRCmJxzwgBEX1XUVgQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:41 +0200 Message-Id: <20211128235752.10836-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 06/17] libcamera: base: file_descriptor: Add constructor from UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Add a FileDescriptor constructor that takes a UniqueFD, transfering ownership of the file descriptor to the FileDescriptor. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- Changes since v2: - Pass UniqueFD by value --- include/libcamera/base/file_descriptor.h | 3 +++ src/libcamera/base/file_descriptor.cpp | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h index 5d1594e80801..74292eba04f5 100644 --- a/include/libcamera/base/file_descriptor.h +++ b/include/libcamera/base/file_descriptor.h @@ -11,11 +11,14 @@ namespace libcamera { +class UniqueFD; + class FileDescriptor final { public: explicit FileDescriptor(const int &fd = -1); explicit FileDescriptor(int &&fd); + explicit FileDescriptor(UniqueFD fd); FileDescriptor(const FileDescriptor &other); FileDescriptor(FileDescriptor &&other); ~FileDescriptor(); diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp index 98d4b4bfd24f..da696b2501cd 100644 --- a/src/libcamera/base/file_descriptor.cpp +++ b/src/libcamera/base/file_descriptor.cpp @@ -13,6 +13,7 @@ #include #include +#include /** * \file base/file_descriptor.h @@ -109,6 +110,18 @@ FileDescriptor::FileDescriptor(int &&fd) fd = -1; } +/** + * \brief Create a FileDescriptor taking ownership of a given UniqueFD \a fd + * \param[in] fd UniqueFD + * + * Construct a FileDescriptor from UniqueFD by taking ownership of the \a fd. + * The original \a fd becomes invalid. + */ +FileDescriptor::FileDescriptor(UniqueFD fd) + : FileDescriptor(fd.release()) +{ +} + /** * \brief Copy constructor, create a FileDescriptor from a copy of \a other * \param[in] other The other FileDescriptor From patchwork Sun Nov 28 23:57:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14822 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2D472BF415 for ; Sun, 28 Nov 2021 23:58:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 336E6605A6; Mon, 29 Nov 2021 00:58:31 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mDUSoE4P"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B88C6057B for ; Mon, 29 Nov 2021 00:58:24 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EC727730; Mon, 29 Nov 2021 00:58:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143904; bh=6VQC/oNlDxzmvYqn6LpHYeUHO8aITJVndH01oUYvOAM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mDUSoE4PHpHJdv5X/hD5mKwkBaEGuHn05hwxJv/4gLt1fxodFD/+a7FLhooVvKZB4 3BaHL5keoadSydOlFMCm46rvif1l9JIa4QsGG8nGL8oL5BKmMTuHqwk0JfDHi9oW+x /DXqW3aaw2wdUbqU6zxEbMX3/WqXFM2p5hhPcM1w= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:42 +0200 Message-Id: <20211128235752.10836-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 07/17] libcamera: base: file_descriptor: Return UniqueFD from dup() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The dup() function returns a duplicate of the file descriptor. Wrapping it in a FileDescriptor isn't wrong as such, but it prevents from using it in contexts where a UniqueFD is needed. As the duplicate is guaranteed to have a single owner when created, return it as a UniqueFD instead. A FileDescriptor can easily be created from the UniqueFD if desired. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/base/file_descriptor.h | 2 +- src/libcamera/base/file_descriptor.cpp | 22 ++++++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/file_descriptor.h index 74292eba04f5..12a43f95d414 100644 --- a/include/libcamera/base/file_descriptor.h +++ b/include/libcamera/base/file_descriptor.h @@ -28,7 +28,7 @@ public: bool isValid() const { return fd_ != nullptr; } int fd() const { return fd_ ? fd_->fd() : -1; } - FileDescriptor dup() const; + UniqueFD dup() const; private: class Descriptor diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp index da696b2501cd..a83bf52c31e6 100644 --- a/src/libcamera/base/file_descriptor.cpp +++ b/src/libcamera/base/file_descriptor.cpp @@ -222,17 +222,23 @@ FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) * \brief Duplicate a FileDescriptor * * Duplicating a FileDescriptor creates a duplicate of the wrapped file - * descriptor and returns a new FileDescriptor instance that wraps the - * duplicate. The fd() function of the original and duplicate instances will - * return different values. The duplicate instance will not be affected by - * destruction of the original instance or its copies. + * descriptor and returns a UniqueFD that owns the duplicate. The fd() function + * of the original and the get() function of the duplicate will return different + * values. The duplicate instance will not be affected by destruction of the + * original instance or its copies. * - * \return A new FileDescriptor instance wrapping a duplicate of the original - * file descriptor + * \return A UniqueFD owning a duplicate of the original file descriptor */ -FileDescriptor FileDescriptor::dup() const +UniqueFD FileDescriptor::dup() const { - return FileDescriptor(fd()); + int dupFd = ::dup(fd()); + if (dupFd == -1) { + int ret = -errno; + LOG(FileDescriptor, Error) + << "Failed to dup() fd: " << strerror(-ret); + } + + return UniqueFD(dupFd); } FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate) From patchwork Sun Nov 28 23:57:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14823 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 349C2C3251 for ; Sun, 28 Nov 2021 23:58:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AF357605B6; Mon, 29 Nov 2021 00:58:32 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="b9HnCH2F"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D467660596 for ; Mon, 29 Nov 2021 00:58:24 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 60E45A15; Mon, 29 Nov 2021 00:58:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143904; bh=eVMS4rD6rnd3lfEo/ZYM1xExa2nsFuds09Vpy5zarcE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=b9HnCH2FL+Jd2kcmmNJXCO7x07Wg98IRp62FbCbcVFbiubtygr1y7S3XW7sgs/G6k eVRAHwshhHjdUL7DNSjOvSDU9nlF5tCLmw+M1qbULfo+UNnkkegXScwtKDOtd8TE1E z3+/8NkqaYse8i8CPg5/1hEtQJKIHqp7KSToG48g= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:43 +0200 Message-Id: <20211128235752.10836-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 08/17] libcamera: event_dispatcher_poll: Manage fd by UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages the event file descriptor owned by EventDispatcherPoll by UniqueFD. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/base/event_dispatcher_poll.h | 3 ++- src/libcamera/base/event_dispatcher_poll.cpp | 11 +++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/libcamera/base/event_dispatcher_poll.h b/include/libcamera/base/event_dispatcher_poll.h index 3d2fc7bbc4c3..b7840309bb25 100644 --- a/include/libcamera/base/event_dispatcher_poll.h +++ b/include/libcamera/base/event_dispatcher_poll.h @@ -14,6 +14,7 @@ #include #include +#include struct pollfd; @@ -50,7 +51,7 @@ private: std::map notifiers_; std::list timers_; - int eventfd_; + UniqueFD eventfd_; bool processingEvents_; }; diff --git a/src/libcamera/base/event_dispatcher_poll.cpp b/src/libcamera/base/event_dispatcher_poll.cpp index 3c9a126c0bd6..8ee22d5adcc4 100644 --- a/src/libcamera/base/event_dispatcher_poll.cpp +++ b/src/libcamera/base/event_dispatcher_poll.cpp @@ -54,14 +54,13 @@ EventDispatcherPoll::EventDispatcherPoll() * Create the event fd. Failures are fatal as we can't implement an * interruptible dispatcher without the fd. */ - eventfd_ = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); - if (eventfd_ < 0) + eventfd_ = UniqueFD(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK)); + if (!eventfd_.isValid()) LOG(Event, Fatal) << "Unable to create eventfd"; } EventDispatcherPoll::~EventDispatcherPoll() { - close(eventfd_); } void EventDispatcherPoll::registerEventNotifier(EventNotifier *notifier) @@ -154,7 +153,7 @@ void EventDispatcherPoll::processEvents() for (auto notifier : notifiers_) pollfds.push_back({ notifier.first, notifier.second.events(), 0 }); - pollfds.push_back({ eventfd_, POLLIN, 0 }); + pollfds.push_back({ eventfd_.get(), POLLIN, 0 }); /* Wait for events and process notifiers and timers. */ do { @@ -176,7 +175,7 @@ void EventDispatcherPoll::processEvents() void EventDispatcherPoll::interrupt() { uint64_t value = 1; - ssize_t ret = write(eventfd_, &value, sizeof(value)); + ssize_t ret = write(eventfd_.get(), &value, sizeof(value)); if (ret != sizeof(value)) { if (ret < 0) ret = -errno; @@ -230,7 +229,7 @@ void EventDispatcherPoll::processInterrupt(const struct pollfd &pfd) return; uint64_t value; - ssize_t ret = read(eventfd_, &value, sizeof(value)); + ssize_t ret = read(eventfd_.get(), &value, sizeof(value)); if (ret != sizeof(value)) { if (ret < 0) ret = -errno; From patchwork Sun Nov 28 23:57:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14824 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 33742C3250 for ; Sun, 28 Nov 2021 23:58:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B4C3860596; Mon, 29 Nov 2021 00:58:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="YbgM20Pc"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 46ECF6011D for ; Mon, 29 Nov 2021 00:58:25 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C9E11730; Mon, 29 Nov 2021 00:58:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143905; bh=zxxSb4XXJYUx3mNsv1S+NucKWSFlybKznq1uDPLBFFw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YbgM20PcObY7CfH02aq5LsHJh2Bx8HaWguhpdKvg25eiCqqMe85MY/T04UHQEgUUJ z0Oe/0Rsn7yOKunpEAxTAWJiIT7ZHi9QSBxO4eDXL28qUyHP4GPHznQ25nFfqLLi5x 05OqPDZ0vQCIqML+ffdUQ0TbznFz8Btv0/IelozA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:44 +0200 Message-Id: <20211128235752.10836-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 09/17] libcamera: file: Manage fd by UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages the file descriptor owned by File by UniqueFD. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/base/file.h | 5 +++-- src/libcamera/base/file.cpp | 25 ++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h index 996751a7ab72..47769da7abc2 100644 --- a/include/libcamera/base/file.h +++ b/include/libcamera/base/file.h @@ -17,6 +17,7 @@ #include #include #include +#include namespace libcamera { @@ -50,7 +51,7 @@ public: bool exists() const; bool open(OpenMode mode); - bool isOpen() const { return fd_ != -1; } + bool isOpen() const { return fd_.isValid(); } OpenMode openMode() const { return mode_; } void close(); @@ -76,7 +77,7 @@ private: void unmapAll(); std::string name_; - int fd_; + UniqueFD fd_; OpenMode mode_; int error_; diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp index 7043f9461cf7..66c73c406198 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -84,7 +84,7 @@ LOG_DEFINE_CATEGORY(File) * before performing I/O operations. */ File::File(const std::string &name) - : name_(name), fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0) + : name_(name), mode_(OpenModeFlag::NotOpen), error_(0) { } @@ -95,7 +95,7 @@ File::File(const std::string &name) * setFileName(). */ File::File() - : fd_(-1), mode_(OpenModeFlag::NotOpen), error_(0) + : mode_(OpenModeFlag::NotOpen), error_(0) { } @@ -178,8 +178,8 @@ bool File::open(File::OpenMode mode) if (mode & OpenModeFlag::WriteOnly) flags |= O_CREAT; - fd_ = ::open(name_.c_str(), flags, 0666); - if (fd_ < 0) { + fd_ = UniqueFD(::open(name_.c_str(), flags, 0666)); + if (!fd_.isValid()) { error_ = -errno; return false; } @@ -210,11 +210,10 @@ bool File::open(File::OpenMode mode) */ void File::close() { - if (fd_ == -1) + if (!fd_.isValid()) return; - ::close(fd_); - fd_ = -1; + fd_.reset(); mode_ = OpenModeFlag::NotOpen; } @@ -244,7 +243,7 @@ ssize_t File::size() const return -EINVAL; struct stat st; - int ret = fstat(fd_, &st); + int ret = fstat(fd_.get(), &st); if (ret < 0) return -errno; @@ -263,7 +262,7 @@ off_t File::pos() const if (!isOpen()) return 0; - return lseek(fd_, 0, SEEK_CUR); + return lseek(fd_.get(), 0, SEEK_CUR); } /** @@ -277,7 +276,7 @@ off_t File::seek(off_t pos) if (!isOpen()) return -EINVAL; - off_t ret = lseek(fd_, pos, SEEK_SET); + off_t ret = lseek(fd_.get(), pos, SEEK_SET); if (ret < 0) return -errno; @@ -309,7 +308,7 @@ ssize_t File::read(const Span &data) /* Retry in case of interrupted system calls. */ while (readBytes < data.size()) { - ret = ::read(fd_, data.data() + readBytes, + ret = ::read(fd_.get(), data.data() + readBytes, data.size() - readBytes); if (ret <= 0) break; @@ -346,7 +345,7 @@ ssize_t File::write(const Span &data) /* Retry in case of interrupted system calls. */ while (writtenBytes < data.size()) { - ssize_t ret = ::write(fd_, data.data() + writtenBytes, + ssize_t ret = ::write(fd_.get(), data.data() + writtenBytes, data.size() - writtenBytes); if (ret <= 0) break; @@ -409,7 +408,7 @@ Span File::map(off_t offset, ssize_t size, File::MapFlags flags) if (flags & MapFlag::Private) prot |= PROT_WRITE; - void *map = mmap(NULL, size, prot, mmapFlags, fd_, offset); + void *map = mmap(NULL, size, prot, mmapFlags, fd_.get(), offset); if (map == MAP_FAILED) { error_ = -errno; return {}; From patchwork Sun Nov 28 23:57:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14825 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C5D2BC324F for ; Sun, 28 Nov 2021 23:58:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6CDD0605B8; Mon, 29 Nov 2021 00:58:34 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GFP9horl"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B8D786059C for ; Mon, 29 Nov 2021 00:58:25 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 49EF4A15; Mon, 29 Nov 2021 00:58:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143905; bh=FcuYDBN8znoPPe2i1sr31rqVNT8kLvY4+JAAGcrq4YY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GFP9horlufSrGjLQFZnUr5d10BtcP2qzLlkzxNYTdSVIRUcwsTgmEBawIISS0Uw4u lhsD53IDeUiIrnVEFgRCEATSKlIrspQo8ifsS/D8JyDZYU1s2ov9lD9SiFy25tKP54 jAv1vUkaZGkTGecySl62RxAZmzmzHntZZft2VrBY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:45 +0200 Message-Id: <20211128235752.10836-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 10/17] libcamera: ipc_unixsocket: Use UniqueFD for a file descriptor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda IPCUnixSocket::create() creates two file descriptors. One of them is stored in IPCUnixSocket and the other is returned to a caller. This clarifies the ownership using UniqueFD. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda if applicable. --- Changes since v2: - Pass UniqueFD to IPCUnixSocket::bind() - Return {} --- include/libcamera/internal/ipc_unixsocket.h | 7 +-- src/libcamera/ipc_pipe_unixsocket.cpp | 8 ++-- src/libcamera/ipc_unixsocket.cpp | 43 ++++++++++--------- test/ipc/unixsocket.cpp | 14 +++--- test/ipc/unixsocket_ipc.cpp | 8 ++-- .../module_ipa_proxy_worker.cpp.tmpl | 13 +++--- 6 files changed, 49 insertions(+), 44 deletions(-) diff --git a/include/libcamera/internal/ipc_unixsocket.h b/include/libcamera/internal/ipc_unixsocket.h index 5010b66a2bda..3963d182ffa6 100644 --- a/include/libcamera/internal/ipc_unixsocket.h +++ b/include/libcamera/internal/ipc_unixsocket.h @@ -12,6 +12,7 @@ #include #include +#include namespace libcamera { @@ -28,8 +29,8 @@ public: IPCUnixSocket(); ~IPCUnixSocket(); - int create(); - int bind(int fd); + UniqueFD create(); + int bind(UniqueFD fd); void close(); bool isBound() const; @@ -49,7 +50,7 @@ private: void dataNotifier(); - int fd_; + UniqueFD fd_; bool headerReceived_; struct Header header_; EventNotifier *notifier_; diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp index 533560cf95d3..65277500ff42 100644 --- a/src/libcamera/ipc_pipe_unixsocket.cpp +++ b/src/libcamera/ipc_pipe_unixsocket.cpp @@ -31,14 +31,14 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath, args.push_back(ipaModulePath); socket_ = std::make_unique(); - int fd = socket_->create(); - if (fd < 0) { + UniqueFD fd = socket_->create(); + if (!fd.isValid()) { LOG(IPCPipe, Error) << "Failed to create socket"; return; } socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead); - args.push_back(std::to_string(fd)); - fds.push_back(fd); + args.push_back(std::to_string(fd.get())); + fds.push_back(fd.release()); proc_ = std::make_unique(); int ret = proc_->start(ipaProxyWorkerPath, args, fds); diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index bd32fca3a678..1980d374cea8 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -7,6 +7,7 @@ #include "libcamera/internal/ipc_unixsocket.h" +#include #include #include #include @@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket) */ IPCUnixSocket::IPCUnixSocket() - : fd_(-1), headerReceived_(false), notifier_(nullptr) + : headerReceived_(false), notifier_(nullptr) { } @@ -86,9 +87,9 @@ IPCUnixSocket::~IPCUnixSocket() * to the remote process, where it can be used with IPCUnixSocket::bind() to * bind the remote side socket. * - * \return A file descriptor on success, negative error code on failure + * \return A file descriptor. It is valid on success or invalid otherwise. */ -int IPCUnixSocket::create() +UniqueFD IPCUnixSocket::create() { int sockets[2]; int ret; @@ -98,14 +99,18 @@ int IPCUnixSocket::create() ret = -errno; LOG(IPCUnixSocket, Error) << "Failed to create socket pair: " << strerror(-ret); - return ret; + return {}; } - ret = bind(sockets[0]); - if (ret) - return ret; + std::array socketFds{ + UniqueFD(sockets[0]), + UniqueFD(sockets[1]), + }; - return sockets[1]; + if (bind(std::move(socketFds[0])) < 0) + return {}; + + return std::move(socketFds[1]); } /** @@ -118,13 +123,13 @@ int IPCUnixSocket::create() * * \return 0 on success or a negative error code otherwise */ -int IPCUnixSocket::bind(int fd) +int IPCUnixSocket::bind(UniqueFD fd) { if (isBound()) return -EINVAL; - fd_ = fd; - notifier_ = new EventNotifier(fd_, EventNotifier::Read); + fd_ = std::move(fd); + notifier_ = new EventNotifier(fd_.get(), EventNotifier::Read); notifier_->activated.connect(this, &IPCUnixSocket::dataNotifier); return 0; @@ -143,9 +148,7 @@ void IPCUnixSocket::close() delete notifier_; notifier_ = nullptr; - ::close(fd_); - - fd_ = -1; + fd_.reset(); headerReceived_ = false; } @@ -155,7 +158,7 @@ void IPCUnixSocket::close() */ bool IPCUnixSocket::isBound() const { - return fd_ != -1; + return fd_.isValid(); } /** @@ -182,7 +185,7 @@ int IPCUnixSocket::send(const Payload &payload) if (!hdr.data && !hdr.fds) return -EINVAL; - ret = ::send(fd_, &hdr, sizeof(hdr), 0); + ret = ::send(fd_.get(), &hdr, sizeof(hdr), 0); if (ret < 0) { ret = -errno; LOG(IPCUnixSocket, Error) @@ -263,7 +266,7 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, if (fds) memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); - if (sendmsg(fd_, &msg, 0) < 0) { + if (sendmsg(fd_.get(), &msg, 0) < 0) { int ret = -errno; LOG(IPCUnixSocket, Error) << "Failed to sendmsg: " << strerror(-ret); @@ -297,7 +300,7 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, msg.msg_controllen = cmsg->cmsg_len; msg.msg_flags = 0; - if (recvmsg(fd_, &msg, 0) < 0) { + if (recvmsg(fd_.get(), &msg, 0) < 0) { int ret = -errno; if (ret != -EAGAIN) LOG(IPCUnixSocket, Error) @@ -317,7 +320,7 @@ void IPCUnixSocket::dataNotifier() if (!headerReceived_) { /* Receive the header. */ - ret = ::recv(fd_, &header_, sizeof(header_), 0); + ret = ::recv(fd_.get(), &header_, sizeof(header_), 0); if (ret < 0) { ret = -errno; LOG(IPCUnixSocket, Error) @@ -333,7 +336,7 @@ void IPCUnixSocket::dataNotifier() * readyRead signal. The notifier will be reenabled by the receive() * function. */ - struct pollfd fds = { fd_, POLLIN, 0 }; + struct pollfd fds = { fd_.get(), POLLIN, 0 }; ret = poll(&fds, 1, 0); if (ret < 0) return; diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp index 7270bf4d2fe7..414f1bfc9d12 100644 --- a/test/ipc/unixsocket.cpp +++ b/test/ipc/unixsocket.cpp @@ -52,9 +52,9 @@ public: ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead); } - int run(int fd) + int run(UniqueFD fd) { - if (ipc_.bind(fd)) { + if (ipc_.bind(std::move(fd))) { cerr << "Failed to connect to IPC channel" << endl; return EXIT_FAILURE; } @@ -360,11 +360,11 @@ protected: int run() { - int slavefd = ipc_.create(); - if (slavefd < 0) + UniqueFD slavefd = ipc_.create(); + if (!slavefd.isValid()) return TestFail; - if (slaveStart(slavefd)) { + if (slaveStart(slavefd.release())) { cerr << "Failed to start slave" << endl; return TestFail; } @@ -496,9 +496,9 @@ private: int main(int argc, char **argv) { if (argc == 2) { - int ipcfd = std::stoi(argv[1]); + UniqueFD ipcfd = UniqueFD(std::stoi(argv[1])); UnixSocketTestSlave slave; - return slave.run(ipcfd); + return slave.run(std::move(ipcfd)); } return UnixSocketTest().execute(); diff --git a/test/ipc/unixsocket_ipc.cpp b/test/ipc/unixsocket_ipc.cpp index ab5d25572d83..178ee1870056 100644 --- a/test/ipc/unixsocket_ipc.cpp +++ b/test/ipc/unixsocket_ipc.cpp @@ -49,9 +49,9 @@ public: ipc_.readyRead.connect(this, &UnixSocketTestIPCSlave::readyRead); } - int run(int fd) + int run(UniqueFD fd) { - if (ipc_.bind(fd)) { + if (ipc_.bind(std::move(fd))) { cerr << "Failed to connect to IPC channel" << endl; return EXIT_FAILURE; } @@ -222,9 +222,9 @@ int main(int argc, char **argv) { /* IPCPipeUnixSocket passes IPA module path in argv[1] */ if (argc == 3) { - int ipcfd = std::stoi(argv[2]); + UniqueFD ipcfd = UniqueFD(std::stoi(argv[2])); UnixSocketTestIPCSlave slave; - return slave.run(ipcfd); + return slave.run(std::move(ipcfd)); } return UnixSocketTestIPC().execute(); diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl index 764e7a3af63a..b65dc4cf31c5 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl @@ -27,8 +27,9 @@ #include #include -#include #include +#include +#include #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/control_serializer.h" @@ -122,9 +123,9 @@ public: } } - int init(std::unique_ptr &ipam, int socketfd) + int init(std::unique_ptr &ipam, UniqueFD socketfd) { - if (socket_.bind(socketfd) < 0) { + if (socket_.bind(std::move(socketfd)) < 0) { LOG({{proxy_worker_name}}, Error) << "IPC socket binding failed"; return EXIT_FAILURE; @@ -203,10 +204,10 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - int fd = std::stoi(argv[2]); + UniqueFD fd(std::stoi(argv[2])); LOG({{proxy_worker_name}}, Info) << "Starting worker for IPA module " << argv[1] - << " with IPC fd = " << fd; + << " with IPC fd = " << fd.get(); std::unique_ptr ipam = std::make_unique(argv[1]); if (!ipam->isValid() || !ipam->load()) { @@ -228,7 +229,7 @@ int main(int argc, char **argv) } {{proxy_worker_name}} proxyWorker; - int ret = proxyWorker.init(ipam, fd); + int ret = proxyWorker.init(ipam, std::move(fd)); if (ret < 0) { LOG({{proxy_worker_name}}, Error) << "Failed to initialize proxy worker"; From patchwork Sun Nov 28 23:57:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14826 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5B97CC3252 for ; Sun, 28 Nov 2021 23:58:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0AF766059C; Mon, 29 Nov 2021 00:58:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GjiQOx04"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3378C60593 for ; Mon, 29 Nov 2021 00:58:26 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B4E50730; Mon, 29 Nov 2021 00:58:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143906; bh=wZUeBApQnhJ+XbrzQ/BP6VCbs3sZ06zFDy7nWoacw54=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GjiQOx04Vo9jHgUhdb4NQOIJTet0bQNVseqw/8Q2u48FXRjGw6k358odGPldOyHOl elZeFRHQdI/p1YMLBg5O+lvsrrnWtQMyWT7VU5b3TnTVeXan5y/DniiA9beyXbwyY5 cW1/yzn0cNR5v1xHAMWgq9gZzuajYW3HJDQ/3yK8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:46 +0200 Message-Id: <20211128235752.10836-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 11/17] libcamera: process: Manage pipe fds by UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages the file descriptors owned by Process for pipe by UniqueFDs. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- include/libcamera/internal/process.h | 4 +++- src/libcamera/process.cpp | 16 ++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 96748a3676e4..95e67e105a92 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -12,6 +12,7 @@ #include #include +#include namespace libcamera { @@ -75,8 +76,9 @@ private: std::list processes_; struct sigaction oldsa_; + EventNotifier *sigEvent_; - int pipe_[2]; + UniqueFD pipe_[2]; }; } /* namespace libcamera */ diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index eca1b30039b8..0e6b4e1dde58 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -69,7 +69,7 @@ void sigact(int signal, siginfo_t *info, void *ucontext) void ProcessManager::sighandler() { char data; - ssize_t ret = read(pipe_[0], &data, sizeof(data)); + ssize_t ret = read(pipe_[0].get(), &data, sizeof(data)); if (ret < 0) { LOG(Process, Error) << "Failed to read byte from signal handler pipe"; @@ -129,10 +129,15 @@ ProcessManager::ProcessManager() sigaction(SIGCHLD, &sa, NULL); - if (pipe2(pipe_, O_CLOEXEC | O_DIRECT | O_NONBLOCK)) + int pipe[2]; + if (pipe2(pipe, O_CLOEXEC | O_DIRECT | O_NONBLOCK)) LOG(Process, Fatal) << "Failed to initialize pipe for signal handling"; - sigEvent_ = new EventNotifier(pipe_[0], EventNotifier::Read); + + pipe_[0] = UniqueFD(pipe[0]); + pipe_[1] = UniqueFD(pipe[1]); + + sigEvent_ = new EventNotifier(pipe_[0].get(), EventNotifier::Read); sigEvent_->activated.connect(this, &ProcessManager::sighandler); self_ = this; @@ -141,9 +146,8 @@ ProcessManager::ProcessManager() ProcessManager::~ProcessManager() { sigaction(SIGCHLD, &oldsa_, NULL); + delete sigEvent_; - close(pipe_[0]); - close(pipe_[1]); self_ = nullptr; } @@ -170,7 +174,7 @@ ProcessManager *ProcessManager::instance() */ int ProcessManager::writePipe() const { - return pipe_[1]; + return pipe_[1].get(); } /** From patchwork Sun Nov 28 23:57:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14827 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 54467BF415 for ; Sun, 28 Nov 2021 23:58:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0FD50605B4; Mon, 29 Nov 2021 00:58:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TVbfwXR2"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 962006058D for ; Mon, 29 Nov 2021 00:58:26 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D147A15; Mon, 29 Nov 2021 00:58:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143906; bh=bQm8dziWNc1hC7aP8dzQAQSlsC8jKJZNDZnwgaPuOu8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TVbfwXR2XMcm0/3fNm8eLXpJvrk4hxo+RGxTgn9SoLp6dhfuoSiKUYa68a8CxzOz3 tLpJ10dRwURVXJU1J5cvXybODAOAuBRmTZOXuqSiIFKw7eut39OrMrAOtBk6Oq5K07 2V/qGWqeFPRja+ULFgk7lSuXU4pAzmmcAmItbBRE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:47 +0200 Message-Id: <20211128235752.10836-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 12/17] libcamera: media_device: Manage fd by UniqueFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages a file descriptor owned by MediaDevice for a media device node by UniqueFD. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda if applicable. Reviewed-by: Jacopo Mondi --- Changes since v2: - Fix errno handling --- include/libcamera/internal/media_device.h | 3 +- src/libcamera/media_device.cpp | 36 ++++++++++------------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index d636e34a8573..af0b25b731eb 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -16,6 +16,7 @@ #include #include +#include #include "libcamera/internal/media_object.h" @@ -82,7 +83,7 @@ private: unsigned int version_; unsigned int hwRevision_; - int fd_; + UniqueFD fd_; bool valid_; bool acquired_; bool lockOwner_; diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index aa93da75c593..4df0a27fe193 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -63,15 +63,14 @@ LOG_DEFINE_CATEGORY(MediaDevice) * populate() before the media graph can be queried. */ MediaDevice::MediaDevice(const std::string &deviceNode) - : deviceNode_(deviceNode), fd_(-1), valid_(false), acquired_(false), + : deviceNode_(deviceNode), valid_(false), acquired_(false), lockOwner_(false) { } MediaDevice::~MediaDevice() { - if (fd_ != -1) - ::close(fd_); + fd_.reset(); clear(); } @@ -143,14 +142,14 @@ void MediaDevice::release() */ bool MediaDevice::lock() { - if (fd_ == -1) + if (!fd_.isValid()) return false; /* Do not allow nested locking in the same libcamera instance. */ if (lockOwner_) return false; - if (lockf(fd_, F_TLOCK, 0)) + if (lockf(fd_.get(), F_TLOCK, 0)) return false; lockOwner_ = true; @@ -169,7 +168,7 @@ bool MediaDevice::lock() */ void MediaDevice::unlock() { - if (fd_ == -1) + if (!fd_.isValid()) return; if (!lockOwner_) @@ -177,7 +176,7 @@ void MediaDevice::unlock() lockOwner_ = false; - lockf(fd_, F_ULOCK, 0); + lockf(fd_.get(), F_ULOCK, 0); } /** @@ -220,7 +219,7 @@ int MediaDevice::populate() return ret; struct media_device_info info = {}; - ret = ioctl(fd_, MEDIA_IOC_DEVICE_INFO, &info); + ret = ioctl(fd_.get(), MEDIA_IOC_DEVICE_INFO, &info); if (ret) { ret = -errno; LOG(MediaDevice, Error) @@ -243,7 +242,7 @@ int MediaDevice::populate() topology.ptr_links = reinterpret_cast(links); topology.ptr_pads = reinterpret_cast(pads); - ret = ioctl(fd_, MEDIA_IOC_G_TOPOLOGY, &topology); + ret = ioctl(fd_.get(), MEDIA_IOC_G_TOPOLOGY, &topology); if (ret < 0) { ret = -errno; LOG(MediaDevice, Error) @@ -481,20 +480,19 @@ int MediaDevice::disableLinks() */ int MediaDevice::open() { - if (fd_ != -1) { + if (fd_.isValid()) { LOG(MediaDevice, Error) << "MediaDevice already open"; return -EBUSY; } - int ret = ::open(deviceNode_.c_str(), O_RDWR); - if (ret < 0) { - ret = -errno; + fd_ = UniqueFD(::open(deviceNode_.c_str(), O_RDWR)); + if (!fd_.isValid()) { + int ret = -errno; LOG(MediaDevice, Error) << "Failed to open media device at " << deviceNode_ << ": " << strerror(-ret); return ret; } - fd_ = ret; return 0; } @@ -514,11 +512,7 @@ int MediaDevice::open() */ void MediaDevice::close() { - if (fd_ == -1) - return; - - ::close(fd_); - fd_ = -1; + fd_.reset(); } /** @@ -756,7 +750,7 @@ void MediaDevice::fixupEntityFlags(struct media_v2_entity *entity) struct media_entity_desc desc = {}; desc.id = entity->id; - int ret = ioctl(fd_, MEDIA_IOC_ENUM_ENTITIES, &desc); + int ret = ioctl(fd_.get(), MEDIA_IOC_ENUM_ENTITIES, &desc); if (ret < 0) { ret = -errno; LOG(MediaDevice, Debug) @@ -799,7 +793,7 @@ int MediaDevice::setupLink(const MediaLink *link, unsigned int flags) linkDesc.flags = flags; - int ret = ioctl(fd_, MEDIA_IOC_SETUP_LINK, &linkDesc); + int ret = ioctl(fd_.get(), MEDIA_IOC_SETUP_LINK, &linkDesc); if (ret) { ret = -errno; LOG(MediaDevice, Error) From patchwork Sun Nov 28 23:57:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14828 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A4D8DC324F for ; Sun, 28 Nov 2021 23:58:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5F457605A1; Mon, 29 Nov 2021 00:58:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="KCVKIXue"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 16CB560592 for ; Mon, 29 Nov 2021 00:58:27 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 971AE730; Mon, 29 Nov 2021 00:58:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143906; bh=jYtZ9x8E5aewctI3eJJsAkDRc86PPPTFhuLcwExvZw4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KCVKIXuevwhuBX6sI8oshhz4F8RbXiMavxreQg75r0zhFF6n4xP+4TSd8gZbTDUGT 8zxbbpgk7xc3RMtfYY12n9HCEK5vS5+CfUTv08chVnPkPsZtxxo9M5FbuCTDePADT1 my4OP7G5UUmJyj/QoN/fDiOwY6tjJShT9XOIph70= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:48 +0200 Message-Id: <20211128235752.10836-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 13/17] libcamera: v4l2_device: Use UniqueFD for a file descriptor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages a file descriptor owned by V4L2Device for a v4l2 device node by UniqueFD. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda if applicable. Reviewed-by: Jacopo Mondi --- Changes since v2: - Fix errno handling --- include/libcamera/internal/v4l2_device.h | 9 +++++---- src/libcamera/v4l2_device.cpp | 23 ++++++++++------------- src/libcamera/v4l2_videodevice.cpp | 16 ++++++---------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 7816a290141d..8886b750ae29 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -16,6 +16,7 @@ #include #include #include +#include #include @@ -27,7 +28,7 @@ class V4L2Device : protected Loggable { public: void close(); - bool isOpen() const { return fd_ != -1; } + bool isOpen() const { return fd_.isValid(); } const ControlInfoMap &controls() const { return controls_; } @@ -49,11 +50,11 @@ protected: ~V4L2Device(); int open(unsigned int flags); - int setFd(int fd); + int setFd(UniqueFD fd); int ioctl(unsigned long request, void *argp); - int fd() const { return fd_; } + int fd() const { return fd_.get(); } private: static ControlType v4l2CtrlType(uint32_t ctrlType); @@ -72,7 +73,7 @@ private: ControlIdMap controlIdMap_; ControlInfoMap controls_; std::string deviceNode_; - int fd_; + UniqueFD fd_; EventNotifier *fdEventNotifier_; bool frameStartEnabled_; diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 9c783c9cbed1..39f360091f64 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -53,7 +53,7 @@ LOG_DEFINE_CATEGORY(V4L2) * at open() time, and the \a logTag to prefix log messages with. */ V4L2Device::V4L2Device(const std::string &deviceNode) - : deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr), + : deviceNode_(deviceNode), fdEventNotifier_(nullptr), frameStartEnabled_(false) { } @@ -81,15 +81,15 @@ int V4L2Device::open(unsigned int flags) return -EBUSY; } - int ret = syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), flags); - if (ret < 0) { - ret = -errno; + UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), flags)); + if (!fd.isValid()) { + int ret = -errno; LOG(V4L2, Error) << "Failed to open V4L2 device: " << strerror(-ret); return ret; } - setFd(ret); + setFd(std::move(fd)); listControls(); @@ -112,14 +112,14 @@ int V4L2Device::open(unsigned int flags) * * \return 0 on success or a negative error code otherwise */ -int V4L2Device::setFd(int fd) +int V4L2Device::setFd(UniqueFD fd) { if (isOpen()) return -EBUSY; - fd_ = fd; + fd_ = std::move(fd); - fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception); + fdEventNotifier_ = new EventNotifier(fd_.get(), EventNotifier::Exception); fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable); fdEventNotifier_->setEnabled(false); @@ -138,10 +138,7 @@ void V4L2Device::close() delete fdEventNotifier_; - if (::close(fd_) < 0) - LOG(V4L2, Error) << "Failed to close V4L2 device: " - << strerror(errno); - fd_ = -1; + fd_.reset(); } /** @@ -440,7 +437,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) * Printing out an error message is usually better performed * in the caller, which can provide more context. */ - if (::ioctl(fd_, request, argp) < 0) + if (::ioctl(fd_.get(), request, argp) < 0) return -errno; return 0; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 0a85bcf6b3ff..c95626d33cc3 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "libcamera/internal/formats.h" @@ -620,22 +621,17 @@ int V4L2VideoDevice::open() */ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) { - int ret; - int newFd; - - newFd = dup(handle); - if (newFd < 0) { - ret = -errno; + UniqueFD newFd(dup(handle)); + if (!newFd.isValid()) { LOG(V4L2, Error) << "Failed to duplicate file handle: " - << strerror(-ret); - return ret; + << strerror(errno); + return -errno; } - ret = V4L2Device::setFd(newFd); + int ret = V4L2Device::setFd(std::move(newFd)); if (ret < 0) { LOG(V4L2, Error) << "Failed to set file handle: " << strerror(-ret); - ::close(newFd); return ret; } From patchwork Sun Nov 28 23:57:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14829 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2E727C3250 for ; Sun, 28 Nov 2021 23:58:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CF2576058C; Mon, 29 Nov 2021 00:58:39 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="i67FNUtK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7422C6057D for ; Mon, 29 Nov 2021 00:58:27 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C71BA15; Mon, 29 Nov 2021 00:58:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143907; bh=KBY2NLCMmHFAHDrxRe2w0oriPC6HklA1vo1zPN9Yn4w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=i67FNUtKOJA/v3b1pe7I3otlBQbwtTjuHGYBrrSZzRZ/RFxJznACG427/Sq+ZCr34 bNz2A9BTeIETVL6ubO2RNwUqPVis9b0TlNpeZKISmd52KOg2vCGMpLVVeyjQSG6rwX gxM3iipHNEDSdxt+SlzVw5H8RWKyokWovpmYfIaE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:49 +0200 Message-Id: <20211128235752.10836-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 14/17] libcamera: v4l2_videodevice: Use fd for a file descriptor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages file descriptors owned by V4L2VideoDevice by UniqueFD. This also changes the return type of exportDmabufFd to UniqueFD from FileDescriptor in order to represent a caller owns the returned file file descriptor. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda if applicable. Reviewed-by: Jacopo Mondi --- Changes since v2: - Use FileDescriptor(UniqueFD &&) constructor - Fix errno handling - Return {} --- include/libcamera/internal/v4l2_videodevice.h | 4 +-- src/libcamera/v4l2_videodevice.cpp | 30 ++++++++----------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 4a44b7fd53b1..80b665771782 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -31,7 +32,6 @@ namespace libcamera { class EventNotifier; -class FileDescriptor; class MediaDevice; class MediaEntity; @@ -238,7 +238,7 @@ private: int createBuffers(unsigned int count, std::vector> *buffers); std::unique_ptr createBuffer(unsigned int index); - FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); + UniqueFD exportDmabufFd(unsigned int index, unsigned int plane); void bufferAvailable(); FrameBuffer *dequeueBuffer(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index c95626d33cc3..3966483a365f 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1320,12 +1320,12 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) std::vector planes; for (unsigned int nplane = 0; nplane < numPlanes; nplane++) { - FileDescriptor fd = exportDmabufFd(buf.index, nplane); + UniqueFD fd = exportDmabufFd(buf.index, nplane); if (!fd.isValid()) return nullptr; FrameBuffer::Plane plane; - plane.fd = std::move(fd); + plane.fd = FileDescriptor(std::move(fd)); /* * V4L2 API doesn't provide dmabuf offset information of plane. * Set 0 as a placeholder offset. @@ -1380,8 +1380,8 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) return std::make_unique(planes); } -FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index, - unsigned int plane) +UniqueFD V4L2VideoDevice::exportDmabufFd(unsigned int index, + unsigned int plane) { struct v4l2_exportbuffer expbuf = {}; int ret; @@ -1395,10 +1395,10 @@ FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index, if (ret < 0) { LOG(V4L2, Error) << "Failed to export buffer: " << strerror(-ret); - return FileDescriptor(); + return {}; } - return FileDescriptor(std::move(expbuf.fd)); + return UniqueFD(expbuf.fd); } /** @@ -1896,7 +1896,6 @@ V4L2M2MDevice::~V4L2M2MDevice() */ int V4L2M2MDevice::open() { - int fd; int ret; /* @@ -1905,30 +1904,27 @@ int V4L2M2MDevice::open() * as the V4L2VideoDevice::open() retains a handle by duplicating the * fd passed in. */ - fd = syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), - O_RDWR | O_NONBLOCK); - if (fd < 0) { + UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), + O_RDWR | O_NONBLOCK)); + if (!fd.isValid()) { ret = -errno; - LOG(V4L2, Error) - << "Failed to open V4L2 M2M device: " << strerror(-ret); + LOG(V4L2, Error) << "Failed to open V4L2 M2M device: " + << strerror(-ret); return ret; } - ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); + ret = output_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_OUTPUT); if (ret) goto err; - ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = capture_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_CAPTURE); if (ret) goto err; - ::close(fd); - return 0; err: close(); - ::close(fd); return ret; } From patchwork Sun Nov 28 23:57:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14830 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id A93CAC3251 for ; Sun, 28 Nov 2021 23:58:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5B2EF605A4; Mon, 29 Nov 2021 00:58:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OsQxKElj"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EDBE7605B8 for ; Mon, 29 Nov 2021 00:58:27 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 75665730; Mon, 29 Nov 2021 00:58:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143907; bh=b+y7AsF0MkaD6dEeLwtDX93/y3Khi4cycGRl3WPGJhU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OsQxKEljP4FdwcSqjgxd9/Xxxw+gJm8gsQd31YVkw16d9sLcIDrPinsplOqqncH/X AVIM/MgcdXrbtiZ8mBQh45sOXtas9vnJ1djnpKgQDnS1ms/VulHeb1782dw+RY+hoD tlniMlPE9iimtmUIMDqK3JdGFwEypbpYgrYhqJ/8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:50 +0200 Message-Id: <20211128235752.10836-16-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 15/17] libcamera: pipeline: raspberrypi: DmaHeaps: Use UniqueFD for a file descriptor X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda Manages a file descriptor owned by DmaHeaps for a dma handle by UniqueFD. Furthermore, DmaHeaps::alloc() creates a new file descriptor and the returned file descriptor is owned by a caller. This also clarifies it by changing the returned value to UniqueFD. Signed-off-by: Hirokazu Honda Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda if applicable. --- Changes since v2: - Use default destructor - Return {} --- .../pipeline/raspberrypi/dma_heaps.cpp | 28 ++++++++----------- .../pipeline/raspberrypi/dma_heaps.h | 10 ++++--- .../pipeline/raspberrypi/raspberrypi.cpp | 6 ++-- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp index 573ea11de607..69831dabbe75 100644 --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp @@ -35,7 +35,6 @@ LOG_DECLARE_CATEGORY(RPI) namespace RPi { DmaHeap::DmaHeap() - : dmaHeapHandle_(-1) { for (const char *name : heapNames) { int ret = ::open(name, O_RDWR, 0); @@ -46,49 +45,44 @@ DmaHeap::DmaHeap() continue; } - dmaHeapHandle_ = ret; + dmaHeapHandle_ = UniqueFD(ret); break; } - if (dmaHeapHandle_ < 0) + if (!dmaHeapHandle_.isValid()) LOG(RPI, Error) << "Could not open any dmaHeap device"; } -DmaHeap::~DmaHeap() -{ - if (dmaHeapHandle_ > -1) - ::close(dmaHeapHandle_); -} +DmaHeap::~DmaHeap() = default; -FileDescriptor DmaHeap::alloc(const char *name, std::size_t size) +UniqueFD DmaHeap::alloc(const char *name, std::size_t size) { int ret; if (!name) - return FileDescriptor(); + return {}; struct dma_heap_allocation_data alloc = {}; alloc.len = size; alloc.fd_flags = O_CLOEXEC | O_RDWR; - ret = ::ioctl(dmaHeapHandle_, DMA_HEAP_IOCTL_ALLOC, &alloc); - + ret = ::ioctl(dmaHeapHandle_.get(), DMA_HEAP_IOCTL_ALLOC, &alloc); if (ret < 0) { LOG(RPI, Error) << "dmaHeap allocation failure for " << name; - return FileDescriptor(); + return {}; } - ret = ::ioctl(alloc.fd, DMA_BUF_SET_NAME, name); + UniqueFD allocFd(alloc.fd); + ret = ::ioctl(allocFd.get(), DMA_BUF_SET_NAME, name); if (ret < 0) { LOG(RPI, Error) << "dmaHeap naming failure for " << name; - ::close(alloc.fd); - return FileDescriptor(); + return {}; } - return FileDescriptor(std::move(alloc.fd)); + return allocFd; } } /* namespace RPi */ diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.h b/src/libcamera/pipeline/raspberrypi/dma_heaps.h index 57beaeb2e48a..d38f41eae1a2 100644 --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.h +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.h @@ -7,7 +7,9 @@ #pragma once -#include +#include + +#include namespace libcamera { @@ -18,11 +20,11 @@ class DmaHeap public: DmaHeap(); ~DmaHeap(); - bool isValid() const { return dmaHeapHandle_ > -1; } - FileDescriptor alloc(const char *name, std::size_t size); + bool isValid() const { return dmaHeapHandle_.isValid(); } + UniqueFD alloc(const char *name, std::size_t size); private: - int dmaHeapHandle_; + UniqueFD dmaHeapHandle_; }; } /* namespace RPi */ diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 7526edf774a2..ffa51a0c65ca 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1361,10 +1361,12 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) /* Allocate the lens shading table via dmaHeap and pass to the IPA. */ if (!lsTable_.isValid()) { - lsTable_ = dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize); - if (!lsTable_.isValid()) + UniqueFD fd = dmaHeap_.alloc("ls_grid", ipa::RPi::MaxLsGridSize); + if (!fd.isValid()) return -ENOMEM; + lsTable_ = FileDescriptor(std::move(fd)); + /* Allow the IPA to mmap the LS table via the file descriptor. */ /* * \todo Investigate if mapping the lens shading table buffer From patchwork Sun Nov 28 23:57:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14831 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3FF32BF415 for ; Sun, 28 Nov 2021 23:58:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DF0C66057D; Mon, 29 Nov 2021 00:58:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FAmLk1QI"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5835C605BA for ; Mon, 29 Nov 2021 00:58:28 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DFD76A15; Mon, 29 Nov 2021 00:58:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143908; bh=sxelJUYeWdlBpBv7l7ZwyQ7eMmEYLFVw22ze63Qx6bg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FAmLk1QIibJPJooFh8BbS8x4chBKjqW7HGCNAByXWLwNSpxeDUPpTi/fneP1iMT4s NgXnid5Z2xFAbgifwSC0NJnQk4ROq5iJtRs7NkaGOjIieiuiYvA5bjBOnNznjBGynz jTnF87l/04mTxEh6PLyuyhqjYwfWNRUY+q6erSLk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:51 +0200 Message-Id: <20211128235752.10836-17-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 16/17] v4l2: v4l2_camera: Return int in getBufferFd() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Hirokazu Honda V4L2Camera::getBufferFd() returns FileDescriptor. However, the file descriptor is still owned by V4L2Camera. It should rather return an integer to represent V4L2Camera doesn't have the ownership of the file descriptor. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/v4l2/v4l2_camera.cpp | 6 +++--- src/v4l2/v4l2_camera.h | 2 +- src/v4l2/v4l2_camera_proxy.cpp | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 157ab94e0544..464507505c79 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -186,16 +186,16 @@ void V4L2Camera::freeBuffers() bufferAllocator_->free(stream); } -FileDescriptor V4L2Camera::getBufferFd(unsigned int index) +int V4L2Camera::getBufferFd(unsigned int index) { Stream *stream = config_->at(0).stream(); const std::vector> &buffers = bufferAllocator_->buffers(stream); if (buffers.size() <= index) - return FileDescriptor(); + return -1; - return buffers[index]->planes()[0].fd; + return buffers[index]->planes()[0].fd.fd(); } int V4L2Camera::streamOn() diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 0cea111561dd..9817fd393d59 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -51,7 +51,7 @@ public: int allocBuffers(unsigned int count); void freeBuffers(); - libcamera::FileDescriptor getBufferFd(unsigned int index); + int getBufferFd(unsigned int index); int streamOn(); int streamOff(); diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 3610e63cade3..627e38d90f5b 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -114,14 +114,14 @@ void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags, return MAP_FAILED; } - FileDescriptor fd = vcam_->getBufferFd(index); - if (!fd.isValid()) { + int fd = vcam_->getBufferFd(index); + if (fd < 0) { errno = EINVAL; return MAP_FAILED; } void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot, - flags, fd.fd(), 0); + flags, fd, 0); if (map == MAP_FAILED) return map; From patchwork Sun Nov 28 23:57:52 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14832 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CD33CC3252 for ; Sun, 28 Nov 2021 23:58:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 76999605B6; Mon, 29 Nov 2021 00:58:41 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="p6R/N11z"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0010E605C5 for ; Mon, 29 Nov 2021 00:58:28 +0100 (CET) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 56AAC730; Mon, 29 Nov 2021 00:58:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638143908; bh=pB/M0LO/tbEsyyRtbOBUvJ8JrbbIWL3/jelq6Q9LXpE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=p6R/N11zwf3HbRqdtNNiyI3zdMAEL6zHs28DydT0mmxaSLbnsLrrjnVtTnFw9aouY jhqoP+ljbCuJCExwW1V/vHS10g0c8wJj3FlTEJsCpdsBYhHSo5l1l52fodvCSXDawp JRrExk5W0SMMZXYeB3hG0ro70okBT+zZUE9OZ7Ws= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Nov 2021 01:57:52 +0200 Message-Id: <20211128235752.10836-18-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> References: <20211128235752.10836-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 17/17] libcamera: base: Rename FileDescriptor to SharedFD X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Now that we have a UniqueFD class, the name FileDescriptor is ambiguous. Rename it to SharedFD. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- include/libcamera/base/file.h | 4 +- include/libcamera/base/meson.build | 2 +- .../base/{file_descriptor.h => shared_fd.h} | 20 +- include/libcamera/framebuffer.h | 4 +- .../libcamera/internal/ipa_data_serializer.h | 40 +-- include/libcamera/internal/ipc_pipe.h | 8 +- include/libcamera/ipa/core.mojom | 6 +- include/libcamera/ipa/raspberrypi.mojom | 2 +- src/android/camera_device.cpp | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 4 +- src/libcamera/base/file.cpp | 6 +- src/libcamera/base/file_descriptor.cpp | 266 ------------------ src/libcamera/base/meson.build | 2 +- src/libcamera/base/shared_fd.cpp | 262 +++++++++++++++++ src/libcamera/framebuffer.cpp | 6 +- src/libcamera/ipa_data_serializer.cpp | 100 +++---- src/libcamera/ipc_pipe.cpp | 4 +- .../pipeline/raspberrypi/raspberrypi.cpp | 6 +- src/libcamera/v4l2_videodevice.cpp | 6 +- src/v4l2/v4l2_camera.h | 2 +- test/meson.build | 2 +- .../ipa_data_serializer_test.cpp | 14 +- test/{file-descriptor.cpp => shared-fd.cpp} | 46 +-- .../module_ipa_proxy.cpp.tmpl | 2 +- .../module_ipa_proxy.h.tmpl | 2 +- .../libcamera_templates/proxy_functions.tmpl | 2 +- .../libcamera_templates/serializer.tmpl | 22 +- .../generators/mojom_libcamera_generator.py | 6 +- 28 files changed, 422 insertions(+), 426 deletions(-) rename include/libcamera/base/{file_descriptor.h => shared_fd.h} (55%) delete mode 100644 src/libcamera/base/file_descriptor.cpp create mode 100644 src/libcamera/base/shared_fd.cpp rename test/{file-descriptor.cpp => shared-fd.cpp} (80%) diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h index 47769da7abc2..691b52d6ab2d 100644 --- a/include/libcamera/base/file.h +++ b/include/libcamera/base/file.h @@ -21,7 +21,7 @@ namespace libcamera { -class FileDescriptor; +class SharedFD; class File { @@ -69,7 +69,7 @@ public: bool unmap(uint8_t *addr); static bool exists(const std::string &name); - static ino_t inode(const FileDescriptor &fd); + static ino_t inode(const SharedFD &fd); private: LIBCAMERA_DISABLE_COPY(File) diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build index cca374a769cc..112420dab225 100644 --- a/include/libcamera/base/meson.build +++ b/include/libcamera/base/meson.build @@ -11,13 +11,13 @@ libcamera_base_headers = files([ 'event_dispatcher_poll.h', 'event_notifier.h', 'file.h', - 'file_descriptor.h', 'flags.h', 'log.h', 'message.h', 'object.h', 'private.h', 'semaphore.h', + 'shared_fd.h', 'signal.h', 'span.h', 'thread.h', diff --git a/include/libcamera/base/file_descriptor.h b/include/libcamera/base/shared_fd.h similarity index 55% rename from include/libcamera/base/file_descriptor.h rename to include/libcamera/base/shared_fd.h index 12a43f95d414..a786885ceb32 100644 --- a/include/libcamera/base/file_descriptor.h +++ b/include/libcamera/base/shared_fd.h @@ -2,7 +2,7 @@ /* * Copyright (C) 2019, Google Inc. * - * file_descriptor.h - File descriptor wrapper + * shared_fd.h - File descriptor wrapper with shared ownership */ #pragma once @@ -13,18 +13,18 @@ namespace libcamera { class UniqueFD; -class FileDescriptor final +class SharedFD final { public: - explicit FileDescriptor(const int &fd = -1); - explicit FileDescriptor(int &&fd); - explicit FileDescriptor(UniqueFD fd); - FileDescriptor(const FileDescriptor &other); - FileDescriptor(FileDescriptor &&other); - ~FileDescriptor(); + explicit SharedFD(const int &fd = -1); + explicit SharedFD(int &&fd); + explicit SharedFD(UniqueFD fd); + SharedFD(const SharedFD &other); + SharedFD(SharedFD &&other); + ~SharedFD(); - FileDescriptor &operator=(const FileDescriptor &other); - FileDescriptor &operator=(FileDescriptor &&other); + SharedFD &operator=(const SharedFD &other); + SharedFD &operator=(SharedFD &&other); bool isValid() const { return fd_ != nullptr; } int fd() const { return fd_ ? fd_->fd() : -1; } diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h index 2fbea9c5be16..357bbe189551 100644 --- a/include/libcamera/framebuffer.h +++ b/include/libcamera/framebuffer.h @@ -13,7 +13,7 @@ #include #include -#include +#include #include namespace libcamera { @@ -51,7 +51,7 @@ class FrameBuffer final : public Extensible public: struct Plane { static constexpr unsigned int kInvalidOffset = std::numeric_limits::max(); - FileDescriptor fd; + SharedFD fd; unsigned int offset = kInvalidOffset; unsigned int length; }; diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h index c2f602d5b7de..a87449c9be48 100644 --- a/include/libcamera/internal/ipa_data_serializer.h +++ b/include/libcamera/internal/ipa_data_serializer.h @@ -66,7 +66,7 @@ template class IPADataSerializer { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const T &data, ControlSerializer *cs = nullptr); static T deserialize(const std::vector &data, @@ -76,12 +76,12 @@ public: ControlSerializer *cs = nullptr); static T deserialize(const std::vector &data, - const std::vector &fds, + const std::vector &fds, ControlSerializer *cs = nullptr); static T deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr); }; @@ -104,11 +104,11 @@ template class IPADataSerializer> { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const std::vector &data, ControlSerializer *cs = nullptr) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; /* Serialize the length. */ uint32_t vecLen = data.size(); @@ -117,7 +117,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector dvec; - std::vector fvec; + std::vector fvec; std::tie(dvec, fvec) = IPADataSerializer::serialize(it, cs); @@ -141,11 +141,11 @@ public: std::vector::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector fds; + std::vector fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::vector deserialize(std::vector &data, std::vector &fds, + static std::vector deserialize(std::vector &data, std::vector &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -153,15 +153,15 @@ public: static std::vector deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { uint32_t vecLen = readPOD(dataBegin, 0, dataEnd); std::vector ret(vecLen); std::vector::const_iterator dataIter = dataBegin + 4; - std::vector::const_iterator fdIter = fdsBegin; + std::vector::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < vecLen; i++) { uint32_t sizeofData = readPOD(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD(dataIter, 4, dataEnd); @@ -201,11 +201,11 @@ template class IPADataSerializer> { public: - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const std::map &data, ControlSerializer *cs = nullptr) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; /* Serialize the length. */ uint32_t mapLen = data.size(); @@ -214,7 +214,7 @@ public: /* Serialize the members. */ for (auto const &it : data) { std::vector dvec; - std::vector fvec; + std::vector fvec; std::tie(dvec, fvec) = IPADataSerializer::serialize(it.first, cs); @@ -247,11 +247,11 @@ public: std::vector::const_iterator dataEnd, ControlSerializer *cs = nullptr) { - std::vector fds; + std::vector fds; return deserialize(dataBegin, dataEnd, fds.cbegin(), fds.end(), cs); } - static std::map deserialize(std::vector &data, std::vector &fds, + static std::map deserialize(std::vector &data, std::vector &fds, ControlSerializer *cs = nullptr) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); @@ -259,8 +259,8 @@ public: static std::map deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { std::map ret; @@ -268,7 +268,7 @@ public: uint32_t mapLen = readPOD(dataBegin, 0, dataEnd); std::vector::const_iterator dataIter = dataBegin + 4; - std::vector::const_iterator fdIter = fdsBegin; + std::vector::const_iterator fdIter = fdsBegin; for (uint32_t i = 0; i < mapLen; i++) { uint32_t sizeofData = readPOD(dataIter, 0, dataEnd); uint32_t sizeofFds = readPOD(dataIter, 4, dataEnd); diff --git a/include/libcamera/internal/ipc_pipe.h b/include/libcamera/internal/ipc_pipe.h index 986f8d886fa6..ab5dd67c3813 100644 --- a/include/libcamera/internal/ipc_pipe.h +++ b/include/libcamera/internal/ipc_pipe.h @@ -9,7 +9,7 @@ #include -#include +#include #include #include "libcamera/internal/ipc_unixsocket.h" @@ -33,17 +33,17 @@ public: Header &header() { return header_; } std::vector &data() { return data_; } - std::vector &fds() { return fds_; } + std::vector &fds() { return fds_; } const Header &header() const { return header_; } const std::vector &data() const { return data_; } - const std::vector &fds() const { return fds_; } + const std::vector &fds() const { return fds_; } private: Header header_; std::vector data_; - std::vector fds_; + std::vector fds_; }; class IPCPipe diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index f7eff0c7ab8c..74f3339e56f2 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -32,7 +32,7 @@ module libcamera; * - This attribute instructs the build system that a (de)serializer is * available for the type and there's no need to generate one * - hasFd - struct fields or empty structs only - * - Designate that this field or empty struct contains a FileDescriptor + * - Designate that this field or empty struct contains a SharedFD * * Rules: * - If the type is defined in a libcamera C++ header *and* a (de)serializer is @@ -60,7 +60,7 @@ module libcamera; * - In mojom, reference the type as FrameBuffer.Plane and only as map/array * member * - [skipHeader] and [skipSerdes] only work here in core.mojom - * - If a field in a struct has a FileDescriptor, but is not explicitly + * - If a field in a struct has a SharedFD, but is not explicitly * defined so in mojom, then the field must be marked with the [hasFd] * attribute * @@ -71,7 +71,7 @@ module libcamera; */ [skipSerdes, skipHeader] struct ControlInfoMap {}; [skipSerdes, skipHeader] struct ControlList {}; -[skipSerdes, skipHeader] struct FileDescriptor {}; +[skipSerdes, skipHeader] struct SharedFD {}; [skipHeader] struct Point { int32 x; diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index e453d46cb14f..acd3cafe6c91 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -35,7 +35,7 @@ struct ISPConfig { struct IPAConfig { uint32 transform; - libcamera.FileDescriptor lsTableHandle; + libcamera.SharedFD lsTableHandle; }; struct StartConfig { diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index f2e0bdbdbbf6..1938b10509fa 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -725,7 +725,7 @@ CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer, std::vector planes(buf.numPlanes()); for (size_t i = 0; i < buf.numPlanes(); ++i) { - FileDescriptor fd{ camera3buffer->data[i] }; + SharedFD fd{ camera3buffer->data[i] }; if (!fd.isValid()) { LOG(HAL, Fatal) << "No valid fd"; return nullptr; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index c6aec09046f7..aaf629eeb3fc 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -15,8 +15,8 @@ #include -#include #include +#include #include #include @@ -164,7 +164,7 @@ private: bool processPending_; /* LS table allocation passed in from the pipeline handler. */ - FileDescriptor lsTableHandle_; + SharedFD lsTableHandle_; void *lsTable_; /* Distinguish the first camera start from others. */ diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp index 66c73c406198..3ca9839bb989 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -14,8 +14,8 @@ #include #include -#include #include +#include /** * \file base/file.h @@ -473,11 +473,11 @@ bool File::exists(const std::string &name) } /** - * \brief Retrieve the inode of a FileDescriptor + * \brief Retrieve the inode of a SharedFD * * \return The file descriptor inode on success, or 0 on error */ -ino_t File::inode(const FileDescriptor &fd) +ino_t File::inode(const SharedFD &fd) { if (!fd.isValid()) return 0; diff --git a/src/libcamera/base/file_descriptor.cpp b/src/libcamera/base/file_descriptor.cpp deleted file mode 100644 index a83bf52c31e6..000000000000 --- a/src/libcamera/base/file_descriptor.cpp +++ /dev/null @@ -1,266 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * file_descriptor.cpp - File descriptor wrapper - */ - -#include - -#include -#include -#include -#include - -#include -#include - -/** - * \file base/file_descriptor.h - * \brief File descriptor wrapper - */ - -namespace libcamera { - -LOG_DEFINE_CATEGORY(FileDescriptor) - -/** - * \class FileDescriptor - * \brief RAII-style wrapper for file descriptors - * - * The FileDescriptor class provides RAII-style lifetime management of file - * descriptors with an efficient mechanism for ownership sharing. At its core, - * an internal Descriptor object wraps a file descriptor (expressed as a signed - * integer) with an RAII-style interface. The Descriptor is then implicitly - * shared with all FileDescriptor instances constructed as copies. - * - * When constructed from a numerical file descriptor, the FileDescriptor - * instance either duplicates or takes over the file descriptor: - * - * - The FileDescriptor(const int &) constructor duplicates the numerical file - * descriptor and wraps the duplicate in a Descriptor. The caller is - * responsible for closing the original file descriptor, and the value - * returned by fd() will be different from the value passed to the - * constructor. - * - * - The FileDescriptor(int &&) constructor takes over the numerical file - * descriptor and wraps it in a Descriptor. The caller shall not touch the - * original file descriptor once the function returns, and the value returned - * by fd() will be identical to the value passed to the constructor. - * - * The copy constructor and assignment operator create copies that share the - * Descriptor, while the move versions of those functions additionally make the - * other FileDescriptor invalid. When the last FileDescriptor that references a - * Descriptor is destroyed, the file descriptor is closed. - * - * The numerical file descriptor is available through the fd() function. All - * FileDescriptor instances created as copies of a FileDescriptor will report - * the same fd() value. Callers can perform operations on the fd(), but shall - * never close it manually. - */ - -/** - * \brief Create a FileDescriptor copying a given \a fd - * \param[in] fd File descriptor - * - * Construct a FileDescriptor from a numerical file descriptor by duplicating - * the \a fd, and take ownership of the copy. The original \a fd is left - * untouched, and the caller is responsible for closing it when appropriate. - * The duplicated file descriptor will be closed automatically when all - * FileDescriptor instances that reference it are destroyed. - * - * If the \a fd is negative, the FileDescriptor is constructed as invalid and - * the fd() function will return -1. - */ -FileDescriptor::FileDescriptor(const int &fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, true); - if (fd_->fd() < 0) - fd_.reset(); -} - -/** - * \brief Create a FileDescriptor taking ownership of a given \a fd - * \param[in] fd File descriptor - * - * Construct a FileDescriptor from a numerical file descriptor by taking - * ownership of the \a fd. The original \a fd is set to -1 and shall not be - * touched by the caller anymore. In particular, the caller shall not close the - * original \a fd manually. The duplicated file descriptor will be closed - * automatically when all FileDescriptor instances that reference it are - * destroyed. - * - * If the \a fd is negative, the FileDescriptor is constructed as invalid and - * the fd() function will return -1. - */ -FileDescriptor::FileDescriptor(int &&fd) -{ - if (fd < 0) - return; - - fd_ = std::make_shared(fd, false); - /* - * The Descriptor constructor can't have failed here, as it took over - * the fd without duplicating it. Just set the original fd to -1 to - * implement move semantics. - */ - fd = -1; -} - -/** - * \brief Create a FileDescriptor taking ownership of a given UniqueFD \a fd - * \param[in] fd UniqueFD - * - * Construct a FileDescriptor from UniqueFD by taking ownership of the \a fd. - * The original \a fd becomes invalid. - */ -FileDescriptor::FileDescriptor(UniqueFD fd) - : FileDescriptor(fd.release()) -{ -} - -/** - * \brief Copy constructor, create a FileDescriptor from a copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor implicitly shares ownership of the wrapped file - * descriptor. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - */ -FileDescriptor::FileDescriptor(const FileDescriptor &other) - : fd_(other.fd_) -{ -} - -/** - * \brief Move constructor, create a FileDescriptor by taking over \a other - * \param[in] other The other FileDescriptor - * - * Moving a FileDescriptor moves the reference to the wrapped descriptor owned - * by \a other to the new FileDescriptor. The \a other FileDescriptor is - * invalidated and its fd() function will return -1. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - */ -FileDescriptor::FileDescriptor(FileDescriptor &&other) - : fd_(std::move(other.fd_)) -{ -} - -/** - * \brief Destroy the FileDescriptor instance - * - * Destroying a FileDescriptor instance releases its reference to the wrapped - * descriptor, if any. When the last instance that references a wrapped - * descriptor is destroyed, the file descriptor is automatically closed. - */ -FileDescriptor::~FileDescriptor() -{ -} - -/** - * \brief Copy assignment operator, replace the wrapped file descriptor with a - * copy of \a other - * \param[in] other The other FileDescriptor - * - * Copying a FileDescriptor creates a new reference to the wrapped file - * descriptor owner by \a other. If \a other is invalid, *this will also be - * invalid. The original FileDescriptor is left untouched, and the caller is - * responsible for destroying it when appropriate. The wrapped file descriptor - * will be closed automatically when all FileDescriptor instances that - * reference it are destroyed. - * - * \return A reference to this FileDescriptor - */ -FileDescriptor &FileDescriptor::operator=(const FileDescriptor &other) -{ - fd_ = other.fd_; - - return *this; -} - -/** - * \brief Move assignment operator, replace the wrapped file descriptor by - * taking over \a other - * \param[in] other The other FileDescriptor - * - * Moving a FileDescriptor moves the reference to the wrapped descriptor owned - * by \a other to the new FileDescriptor. If \a other is invalid, *this will - * also be invalid. The \a other FileDescriptor is invalidated and its fd() - * function will return -1. The wrapped file descriptor will be closed - * automatically when all FileDescriptor instances that reference it are - * destroyed. - * - * \return A reference to this FileDescriptor - */ -FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other) -{ - fd_ = std::move(other.fd_); - - return *this; -} - -/** - * \fn FileDescriptor::isValid() - * \brief Check if the FileDescriptor instance is valid - * \return True if the FileDescriptor is valid, false otherwise - */ - -/** - * \fn FileDescriptor::fd() - * \brief Retrieve the numerical file descriptor - * \return The numerical file descriptor, which may be -1 if the FileDescriptor - * instance is invalid - */ - -/** - * \brief Duplicate a FileDescriptor - * - * Duplicating a FileDescriptor creates a duplicate of the wrapped file - * descriptor and returns a UniqueFD that owns the duplicate. The fd() function - * of the original and the get() function of the duplicate will return different - * values. The duplicate instance will not be affected by destruction of the - * original instance or its copies. - * - * \return A UniqueFD owning a duplicate of the original file descriptor - */ -UniqueFD FileDescriptor::dup() const -{ - int dupFd = ::dup(fd()); - if (dupFd == -1) { - int ret = -errno; - LOG(FileDescriptor, Error) - << "Failed to dup() fd: " << strerror(-ret); - } - - return UniqueFD(dupFd); -} - -FileDescriptor::Descriptor::Descriptor(int fd, bool duplicate) -{ - if (!duplicate) { - fd_ = fd; - return; - } - - /* Failing to dup() a fd should not happen and is fatal. */ - fd_ = ::dup(fd); - if (fd_ == -1) { - int ret = -errno; - LOG(FileDescriptor, Fatal) - << "Failed to dup() fd: " << strerror(-ret); - } -} - -FileDescriptor::Descriptor::~Descriptor() -{ - if (fd_ != -1) - close(fd_); -} - -} /* namespace libcamera */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index b0d85bc19245..ccb746c27466 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -8,12 +8,12 @@ libcamera_base_sources = files([ 'event_dispatcher_poll.cpp', 'event_notifier.cpp', 'file.cpp', - 'file_descriptor.cpp', 'flags.cpp', 'log.cpp', 'message.cpp', 'object.cpp', 'semaphore.cpp', + 'shared_fd.cpp', 'signal.cpp', 'thread.cpp', 'timer.cpp', diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp new file mode 100644 index 000000000000..05b6892f7e19 --- /dev/null +++ b/src/libcamera/base/shared_fd.cpp @@ -0,0 +1,262 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * shared_fd.cpp - File descriptor wrapper with shared ownership + */ + +#include + +#include +#include +#include +#include + +#include +#include + +/** + * \file base/shared_fd.h + * \brief File descriptor wrapper + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(SharedFD) + +/** + * \class SharedFD + * \brief RAII-style wrapper for file descriptors + * + * The SharedFD class provides RAII-style lifetime management of file + * descriptors with an efficient mechanism for ownership sharing. At its core, + * an internal Descriptor object wraps a file descriptor (expressed as a signed + * integer) with an RAII-style interface. The Descriptor is then implicitly + * shared with all SharedFD instances constructed as copies. + * + * When constructed from a numerical file descriptor, the SharedFD instance + * either duplicates or takes over the file descriptor: + * + * - The SharedFD(const int &) constructor duplicates the numerical file + * descriptor and wraps the duplicate in a Descriptor. The caller is + * responsible for closing the original file descriptor, and the value + * returned by fd() will be different from the value passed to the + * constructor. + * + * - The SharedFD(int &&) constructor takes over the numerical file descriptor + * and wraps it in a Descriptor. The caller shall not touch the original file + * descriptor once the function returns, and the value returned by fd() will + * be identical to the value passed to the constructor. + * + * The copy constructor and assignment operator create copies that share the + * Descriptor, while the move versions of those functions additionally make the + * other SharedFD invalid. When the last SharedFD that references a Descriptor + * is destroyed, the file descriptor is closed. + * + * The numerical file descriptor is available through the fd() function. All + * SharedFD instances created as copies of a SharedFD will report the same fd() + * value. Callers can perform operations on the fd(), but shall never close it + * manually. + */ + +/** + * \brief Create a SharedFD copying a given \a fd + * \param[in] fd File descriptor + * + * Construct a SharedFD from a numerical file descriptor by duplicating the + * \a fd, and take ownership of the copy. The original \a fd is left untouched, + * and the caller is responsible for closing it when appropriate. The duplicated + * file descriptor will be closed automatically when all SharedFD instances that + * reference it are destroyed. + * + * If the \a fd is negative, the SharedFD is constructed as invalid and the fd() + * function will return -1. + */ +SharedFD::SharedFD(const int &fd) +{ + if (fd < 0) + return; + + fd_ = std::make_shared(fd, true); + if (fd_->fd() < 0) + fd_.reset(); +} + +/** + * \brief Create a SharedFD taking ownership of a given \a fd + * \param[in] fd File descriptor + * + * Construct a SharedFD from a numerical file descriptor by taking ownership of + * the \a fd. The original \a fd is set to -1 and shall not be touched by the + * caller anymore. In particular, the caller shall not close the original \a fd + * manually. The duplicated file descriptor will be closed automatically when + * all SharedFD instances that reference it are destroyed. + * + * If the \a fd is negative, the SharedFD is constructed as invalid and the fd() + * function will return -1. + */ +SharedFD::SharedFD(int &&fd) +{ + if (fd < 0) + return; + + fd_ = std::make_shared(fd, false); + /* + * The Descriptor constructor can't have failed here, as it took over + * the fd without duplicating it. Just set the original fd to -1 to + * implement move semantics. + */ + fd = -1; +} + +/** + * \brief Create a SharedFD taking ownership of a given UniqueFD \a fd + * \param[in] fd UniqueFD + * + * Construct a SharedFD from UniqueFD by taking ownership of the \a fd. The + * original \a fd becomes invalid. + */ +SharedFD::SharedFD(UniqueFD fd) + : SharedFD(fd.release()) +{ +} + +/** + * \brief Copy constructor, create a SharedFD from a copy of \a other + * \param[in] other The other SharedFD + * + * Copying a SharedFD implicitly shares ownership of the wrapped file + * descriptor. The original SharedFD is left untouched, and the caller is + * responsible for destroying it when appropriate. The wrapped file descriptor + * will be closed automatically when all SharedFD instances that reference it + * are destroyed. + */ +SharedFD::SharedFD(const SharedFD &other) + : fd_(other.fd_) +{ +} + +/** + * \brief Move constructor, create a SharedFD by taking over \a other + * \param[in] other The other SharedFD + * + * Moving a SharedFD moves the reference to the wrapped descriptor owned by + * \a other to the new SharedFD. The \a other SharedFD is invalidated and its + * fd() function will return -1. The wrapped file descriptor will be closed + * automatically when all SharedFD instances that reference it are destroyed. + */ +SharedFD::SharedFD(SharedFD &&other) + : fd_(std::move(other.fd_)) +{ +} + +/** + * \brief Destroy the SharedFD instance + * + * Destroying a SharedFD instance releases its reference to the wrapped + * descriptor, if any. When the last instance that references a wrapped + * descriptor is destroyed, the file descriptor is automatically closed. + */ +SharedFD::~SharedFD() +{ +} + +/** + * \brief Copy assignment operator, replace the wrapped file descriptor with a + * copy of \a other + * \param[in] other The other SharedFD + * + * Copying a SharedFD creates a new reference to the wrapped file descriptor + * owner by \a other. If \a other is invalid, *this will also be invalid. The + * original SharedFD is left untouched, and the caller is responsible for + * destroying it when appropriate. The wrapped file descriptor will be closed + * automatically when all SharedFD instances that reference it are destroyed. + * + * \return A reference to this SharedFD + */ +SharedFD &SharedFD::operator=(const SharedFD &other) +{ + fd_ = other.fd_; + + return *this; +} + +/** + * \brief Move assignment operator, replace the wrapped file descriptor by + * taking over \a other + * \param[in] other The other SharedFD + * + * Moving a SharedFD moves the reference to the wrapped descriptor owned by + * \a other to the new SharedFD. If \a other is invalid, *this will also be + * invalid. The \a other SharedFD is invalidated and its fd() function will + * return -1. The wrapped file descriptor will be closed automatically when + * all SharedFD instances that reference it are destroyed. + * + * \return A reference to this SharedFD + */ +SharedFD &SharedFD::operator=(SharedFD &&other) +{ + fd_ = std::move(other.fd_); + + return *this; +} + +/** + * \fn SharedFD::isValid() + * \brief Check if the SharedFD instance is valid + * \return True if the SharedFD is valid, false otherwise + */ + +/** + * \fn SharedFD::fd() + * \brief Retrieve the numerical file descriptor + * \return The numerical file descriptor, which may be -1 if the SharedFD + * instance is invalid + */ + +/** + * \brief Duplicate a SharedFD + * + * Duplicating a SharedFD creates a duplicate of the wrapped file descriptor and + * returns a UniqueFD that owns the duplicate. The fd() function of the original + * and the get() function of the duplicate will return different values. The + * duplicate instance will not be affected by destruction of the original + * instance or its copies. + * + * \return A UniqueFD owning a duplicate of the original file descriptor + */ +UniqueFD SharedFD::dup() const +{ + int dupFd = ::dup(fd()); + if (dupFd == -1) { + int ret = -errno; + LOG(SharedFD, Error) + << "Failed to dup() fd: " << strerror(-ret); + } + + return UniqueFD(dupFd); +} + +SharedFD::Descriptor::Descriptor(int fd, bool duplicate) +{ + if (!duplicate) { + fd_ = fd; + return; + } + + /* Failing to dup() a fd should not happen and is fatal. */ + fd_ = ::dup(fd); + if (fd_ == -1) { + int ret = -errno; + LOG(SharedFD, Fatal) + << "Failed to dup() fd: " << strerror(-ret); + } +} + +SharedFD::Descriptor::~Descriptor() +{ + if (fd_ != -1) + close(fd_); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index f5bcf107d7aa..0a5bf7fdbeb7 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -180,9 +180,9 @@ FrameBuffer::Private::Private() * offset and length. * * To support DMA access, planes are associated with dmabuf objects represented - * by FileDescriptor handles. The Plane class doesn't handle mapping of the - * memory to the CPU, but applications and IPAs may use the dmabuf file - * descriptors to map the plane memory with mmap() and access its contents. + * by SharedFD handles. The Plane class doesn't handle mapping of the memory to + * the CPU, but applications and IPAs may use the dmabuf file descriptors to map + * the plane memory with mmap() and access its contents. * * \todo Specify how an application shall decide whether to use a single or * multiple dmabufs, based on the camera requirements. diff --git a/src/libcamera/ipa_data_serializer.cpp b/src/libcamera/ipa_data_serializer.cpp index 82ec9b20a411..0a259305afa2 100644 --- a/src/libcamera/ipa_data_serializer.cpp +++ b/src/libcamera/ipa_data_serializer.cpp @@ -31,7 +31,7 @@ LOG_DEFINE_CATEGORY(IPADataSerializer) * * \todo Harden the vector and map deserializer * - * \todo For FileDescriptors, instead of storing a validity flag, store an + * \todo For SharedFDs, instead of storing a validity flag, store an * index into the fd array. This will allow us to use views instead of copying. */ @@ -112,7 +112,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() can be used if the object type \a T and its - * members don't have any FileDescriptor. + * members don't have any SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -132,7 +132,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() can be used if the object type \a T and its - * members don't have any FileDescriptor. + * members don't have any SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -143,7 +143,7 @@ namespace { /** * \fn template IPADataSerializer::deserialize( * const std::vector &data, - * const std::vector &fds, + * const std::vector &fds, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -152,7 +152,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() (or the iterator version) must be used if - * the object type \a T or its members contain FileDescriptor. + * the object type \a T or its members contain SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -164,8 +164,8 @@ namespace { * \fn template IPADataSerializer::deserialize( * std::vector::const_iterator dataBegin, * std::vector::const_iterator dataEnd, - * std::vector::const_iterator fdsBegin, - * std::vector::const_iterator fdsEnd, + * std::vector::const_iterator fdsBegin, + * std::vector::const_iterator fdsEnd, * ControlSerializer *cs = nullptr) * \brief Deserialize byte vector and fd vector into an object * \tparam T Type of object to deserialize to @@ -176,7 +176,7 @@ namespace { * \param[in] cs ControlSerializer * * This version of deserialize() (or the vector version) must be used if - * the object type \a T or its members contain FileDescriptor. + * the object type \a T or its members contain SharedFD. * * \a cs is only necessary if the object type \a T or its members contain * ControlList or ControlInfoMap. @@ -189,7 +189,7 @@ namespace { #define DEFINE_POD_SERIALIZER(type) \ \ template<> \ -std::tuple, std::vector> \ +std::tuple, std::vector> \ IPADataSerializer::serialize(const type &data, \ [[maybe_unused]] ControlSerializer *cs) \ { \ @@ -217,7 +217,7 @@ type IPADataSerializer::deserialize(const std::vector &data, \ \ template<> \ type IPADataSerializer::deserialize(const std::vector &data, \ - [[maybe_unused]] const std::vector &fds, \ + [[maybe_unused]] const std::vector &fds, \ ControlSerializer *cs) \ { \ return deserialize(data.cbegin(), data.end(), cs); \ @@ -226,8 +226,8 @@ type IPADataSerializer::deserialize(const std::vector &data, \ template<> \ type IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, \ std::vector::const_iterator dataEnd, \ - [[maybe_unused]] std::vector::const_iterator fdsBegin, \ - [[maybe_unused]] std::vector::const_iterator fdsEnd, \ + [[maybe_unused]] std::vector::const_iterator fdsBegin, \ + [[maybe_unused]] std::vector::const_iterator fdsEnd, \ ControlSerializer *cs) \ { \ return deserialize(dataBegin, dataEnd, cs); \ @@ -251,7 +251,7 @@ DEFINE_POD_SERIALIZER(double) * function parameter serdes). */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const std::string &data, [[maybe_unused]] ControlSerializer *cs) { @@ -278,7 +278,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator template<> std::string IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, [[maybe_unused]] ControlSerializer *cs) { return { data.cbegin(), data.cend() }; @@ -288,8 +288,8 @@ template<> std::string IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { return { dataBegin, dataEnd }; @@ -307,7 +307,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator * be used. The serialized ControlInfoMap will have zero length. */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const ControlList &data, ControlSerializer *cs) { if (!cs) @@ -407,7 +407,7 @@ IPADataSerializer::deserialize(const std::vector &data, template<> ControlList IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -417,8 +417,8 @@ template<> ControlList IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); @@ -431,7 +431,7 @@ IPADataSerializer::deserialize(std::vector::const_iterator * X bytes - Serialized ControlInfoMap (using ControlSerializer) */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const ControlInfoMap &map, ControlSerializer *cs) { @@ -493,7 +493,7 @@ IPADataSerializer::deserialize(const std::vector &data, template<> ControlInfoMap IPADataSerializer::deserialize(const std::vector &data, - [[maybe_unused]] const std::vector &fds, + [[maybe_unused]] const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), cs); @@ -503,30 +503,30 @@ template<> ControlInfoMap IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs) { return deserialize(dataBegin, dataEnd, cs); } /* - * FileDescriptors are serialized into four bytes that tells if the - * FileDescriptor is valid or not. If it is valid, then for serialization - * the fd will be written to the fd vector, or for deserialization the - * fd vector const_iterator will be valid. + * SharedFD instances are serialized into four bytes that tells if the SharedFD + * is valid or not. If it is valid, then for serialization the fd will be + * written to the fd vector, or for deserialization the fd vector const_iterator + * will be valid. * * This validity is necessary so that we don't send -1 fd over sendmsg(). It * also allows us to simply send the entire fd vector into the deserializer * and it will be recursively consumed as necessary. */ template<> -std::tuple, std::vector> -IPADataSerializer::serialize(const FileDescriptor &data, - [[maybe_unused]] ControlSerializer *cs) +std::tuple, std::vector> +IPADataSerializer::serialize(const SharedFD &data, + [[maybe_unused]] ControlSerializer *cs) { std::vector dataVec; - std::vector fdVec; + std::vector fdVec; /* * Store as uint32_t to prepare for conversion from validity flag @@ -542,11 +542,11 @@ IPADataSerializer::serialize(const FileDescriptor &data, } template<> -FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin, - [[maybe_unused]] std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, - [[maybe_unused]] ControlSerializer *cs) +SharedFD IPADataSerializer::deserialize([[maybe_unused]] std::vector::const_iterator dataBegin, + [[maybe_unused]] std::vector::const_iterator dataEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, + [[maybe_unused]] ControlSerializer *cs) { ASSERT(std::distance(dataBegin, dataEnd) >= 4); @@ -554,13 +554,13 @@ FileDescriptor IPADataSerializer::deserialize([[maybe_unused]] s ASSERT(!(valid && std::distance(fdsBegin, fdsEnd) < 1)); - return valid ? *fdsBegin : FileDescriptor(); + return valid ? *fdsBegin : SharedFD(); } template<> -FileDescriptor IPADataSerializer::deserialize(const std::vector &data, - const std::vector &fds, - [[maybe_unused]] ControlSerializer *cs) +SharedFD IPADataSerializer::deserialize(const std::vector &data, + const std::vector &fds, + [[maybe_unused]] ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end()); } @@ -568,22 +568,22 @@ FileDescriptor IPADataSerializer::deserialize(const std::vector< /* * FrameBuffer::Plane is serialized as: * - * 4 byte - FileDescriptor + * 4 byte - SharedFD * 4 bytes - uint32_t Offset * 4 bytes - uint32_t Length */ template<> -std::tuple, std::vector> +std::tuple, std::vector> IPADataSerializer::serialize(const FrameBuffer::Plane &data, [[maybe_unused]] ControlSerializer *cs) { std::vector dataVec; - std::vector fdsVec; + std::vector fdsVec; std::vector fdBuf; - std::vector fdFds; + std::vector fdFds; std::tie(fdBuf, fdFds) = - IPADataSerializer::serialize(data.fd); + IPADataSerializer::serialize(data.fd); dataVec.insert(dataVec.end(), fdBuf.begin(), fdBuf.end()); fdsVec.insert(fdsVec.end(), fdFds.begin(), fdFds.end()); @@ -597,13 +597,13 @@ template<> FrameBuffer::Plane IPADataSerializer::deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, [[maybe_unused]] ControlSerializer *cs) { FrameBuffer::Plane ret; - ret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4, + ret.fd = IPADataSerializer::deserialize(dataBegin, dataBegin + 4, fdsBegin, fdsBegin + 1); ret.offset = readPOD(dataBegin, 4, dataEnd); ret.length = readPOD(dataBegin, 8, dataEnd); @@ -614,7 +614,7 @@ IPADataSerializer::deserialize(std::vector::const_i template<> FrameBuffer::Plane IPADataSerializer::deserialize(const std::vector &data, - const std::vector &fds, + const std::vector &fds, ControlSerializer *cs) { return deserialize(data.cbegin(), data.end(), fds.cbegin(), fds.end(), cs); diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp index ad870fd4137f..3b47032de0a2 100644 --- a/src/libcamera/ipc_pipe.cpp +++ b/src/libcamera/ipc_pipe.cpp @@ -87,7 +87,7 @@ IPCMessage::IPCMessage(IPCUnixSocket::Payload &payload) data_ = std::vector(payload.data.begin() + sizeof(header_), payload.data.end()); for (int32_t &fd : payload.fds) - fds_.push_back(FileDescriptor(std::move(fd))); + fds_.push_back(SharedFD(std::move(fd))); } /** @@ -112,7 +112,7 @@ IPCUnixSocket::Payload IPCMessage::payload() const data_.data(), data_.size()); } - for (const FileDescriptor &fd : fds_) + for (const SharedFD &fd : fds_) payload.fds.push_back(fd.fd()); return payload; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index ffa51a0c65ca..ea8243912a29 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -12,7 +12,7 @@ #include #include -#include +#include #include #include @@ -228,7 +228,7 @@ public: /* DMAHEAP allocation helper. */ RPi::DmaHeap dmaHeap_; - FileDescriptor lsTable_; + SharedFD lsTable_; std::unique_ptr delayedCtrls_; bool sensorMetadata_; @@ -1365,7 +1365,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) if (!fd.isValid()) return -ENOMEM; - lsTable_ = FileDescriptor(std::move(fd)); + lsTable_ = SharedFD(std::move(fd)); /* Allow the IPA to mmap the LS table via the file descriptor. */ /* diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 3966483a365f..97d431071def 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -22,8 +22,8 @@ #include #include -#include #include +#include #include #include @@ -1325,7 +1325,7 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) return nullptr; FrameBuffer::Plane plane; - plane.fd = FileDescriptor(std::move(fd)); + plane.fd = SharedFD(std::move(fd)); /* * V4L2 API doesn't provide dmabuf offset information of plane. * Set 0 as a placeholder offset. @@ -1354,7 +1354,7 @@ std::unique_ptr V4L2VideoDevice::createBuffer(unsigned int index) ASSERT(numPlanes == 1u); planes.resize(formatInfo_->numPlanes()); - const FileDescriptor &fd = planes[0].fd; + const SharedFD &fd = planes[0].fd; size_t offset = 0; for (auto [i, plane] : utils::enumerate(planes)) { diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 9817fd393d59..586347829845 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -11,8 +11,8 @@ #include #include -#include #include +#include #include #include diff --git a/test/meson.build b/test/meson.build index 42dfbc1f8ee9..daaa3862cdd6 100644 --- a/test/meson.build +++ b/test/meson.build @@ -40,7 +40,6 @@ internal_tests = [ ['event-dispatcher', 'event-dispatcher.cpp'], ['event-thread', 'event-thread.cpp'], ['file', 'file.cpp'], - ['file-descriptor', 'file-descriptor.cpp'], ['flags', 'flags.cpp'], ['hotplug-cameras', 'hotplug-cameras.cpp'], ['mapped-buffer', 'mapped-buffer.cpp'], @@ -49,6 +48,7 @@ internal_tests = [ ['object-delete', 'object-delete.cpp'], ['object-invoke', 'object-invoke.cpp'], ['pixel-format', 'pixel-format.cpp'], + ['shared-fd', 'shared-fd.cpp'], ['signal-threads', 'signal-threads.cpp'], ['threads', 'threads.cpp'], ['timer', 'timer.cpp'], diff --git a/test/serialization/ipa_data_serializer_test.cpp b/test/serialization/ipa_data_serializer_test.cpp index 5fcdcb8eae92..d2050a868b38 100644 --- a/test/serialization/ipa_data_serializer_test.cpp +++ b/test/serialization/ipa_data_serializer_test.cpp @@ -53,7 +53,7 @@ template int testPodSerdes(T in) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer::serialize(in); T out = IPADataSerializer::deserialize(buf, fds); @@ -72,7 +72,7 @@ int testVectorSerdes(const std::vector &in, ControlSerializer *cs = nullptr) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer>::serialize(in, cs); std::vector out = IPADataSerializer>::deserialize(buf, fds, cs); @@ -92,7 +92,7 @@ int testMapSerdes(const std::map &in, ControlSerializer *cs = nullptr) { std::vector buf; - std::vector fds; + std::vector fds; std::tie(buf, fds) = IPADataSerializer>::serialize(in, cs); std::map out = IPADataSerializer>::deserialize(buf, fds, cs); @@ -198,7 +198,7 @@ private: ControlSerializer cs(ControlSerializer::Role::Proxy); /* - * We don't test FileDescriptor serdes because it dup()s, so we + * We don't test SharedFD serdes because it dup()s, so we * can't check for equality. */ std::vector vecUint8 = { 1, 2, 3, 4, 5, 6 }; @@ -219,7 +219,7 @@ private: }; std::vector buf; - std::vector fds; + std::vector fds; if (testVectorSerdes(vecUint8) != TestPass) return TestFail; @@ -291,7 +291,7 @@ private: { { "a", { 1, 2, 3 } }, { "b", { 4, 5, 6 } }, { "c", { 7, 8, 9 } } }; std::vector buf; - std::vector fds; + std::vector fds; if (testMapSerdes(mapUintStr) != TestPass) return TestFail; @@ -359,7 +359,7 @@ private: std::string strEmpty = ""; std::vector buf; - std::vector fds; + std::vector fds; if (testPodSerdes(u32min) != TestPass) return TestFail; diff --git a/test/file-descriptor.cpp b/test/shared-fd.cpp similarity index 80% rename from test/file-descriptor.cpp rename to test/shared-fd.cpp index 76badc4c5fad..60e5d0aaa395 100644 --- a/test/file-descriptor.cpp +++ b/test/shared-fd.cpp @@ -2,7 +2,7 @@ /* * Copyright (C) 2019, Google Inc. * - * file_descriptor.cpp - FileDescriptor test + * shared_fd.cpp - SharedFD test */ #include @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include "test.h" @@ -19,7 +19,7 @@ using namespace libcamera; using namespace std; -class FileDescriptorTest : public Test +class SharedFDTest : public Test { protected: int init() @@ -43,8 +43,8 @@ protected: int run() { - /* Test creating empty FileDescriptor. */ - desc1_ = new FileDescriptor(); + /* Test creating empty SharedFD. */ + desc1_ = new SharedFD(); if (desc1_->fd() != -1) { std::cout << "Failed fd numerical check (default constructor)" @@ -56,10 +56,10 @@ protected: desc1_ = nullptr; /* - * Test creating FileDescriptor by copying numerical file + * Test creating SharedFD by copying numerical file * descriptor. */ - desc1_ = new FileDescriptor(fd_); + desc1_ = new SharedFD(fd_); if (desc1_->fd() == fd_) { std::cout << "Failed fd numerical check (lvalue ref constructor)" << std::endl; @@ -84,13 +84,13 @@ protected: } /* - * Test creating FileDescriptor by taking ownership of + * Test creating SharedFD by taking ownership of * numerical file descriptor. */ int dupFd = dup(fd_); int dupFdCopy = dupFd; - desc1_ = new FileDescriptor(std::move(dupFd)); + desc1_ = new SharedFD(std::move(dupFd)); if (desc1_->fd() != dupFdCopy) { std::cout << "Failed fd numerical check (rvalue ref constructor)" << std::endl; @@ -114,9 +114,9 @@ protected: return TestFail; } - /* Test creating FileDescriptor from other FileDescriptor. */ - desc1_ = new FileDescriptor(fd_); - desc2_ = new FileDescriptor(*desc1_); + /* Test creating SharedFD from other SharedFD. */ + desc1_ = new SharedFD(fd_); + desc2_ = new SharedFD(*desc1_); if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() != desc2_->fd()) { std::cout << "Failed fd numerical check (copy constructor)" @@ -142,10 +142,10 @@ protected: delete desc2_; desc2_ = nullptr; - /* Test creating FileDescriptor by taking over other FileDescriptor. */ - desc1_ = new FileDescriptor(fd_); + /* Test creating SharedFD by taking over other SharedFD. */ + desc1_ = new SharedFD(fd_); fd = desc1_->fd(); - desc2_ = new FileDescriptor(std::move(*desc1_)); + desc2_ = new SharedFD(std::move(*desc1_)); if (desc1_->fd() != -1 || desc2_->fd() != fd) { std::cout << "Failed fd numerical check (move constructor)" @@ -164,9 +164,9 @@ protected: delete desc2_; desc2_ = nullptr; - /* Test creating FileDescriptor by copy assignment. */ - desc1_ = new FileDescriptor(); - desc2_ = new FileDescriptor(fd_); + /* Test creating SharedFD by copy assignment. */ + desc1_ = new SharedFD(); + desc2_ = new SharedFD(fd_); fd = desc2_->fd(); *desc1_ = *desc2_; @@ -188,9 +188,9 @@ protected: delete desc2_; desc2_ = nullptr; - /* Test creating FileDescriptor by move assignment. */ - desc1_ = new FileDescriptor(); - desc2_ = new FileDescriptor(fd_); + /* Test creating SharedFD by move assignment. */ + desc1_ = new SharedFD(); + desc2_ = new SharedFD(fd_); fd = desc2_->fd(); *desc1_ = std::move(*desc2_); @@ -237,7 +237,7 @@ private: int fd_; ino_t inodeNr_; - FileDescriptor *desc1_, *desc2_; + SharedFD *desc1_, *desc2_; }; -TEST_REGISTER(FileDescriptorTest) +TEST_REGISTER(SharedFDTest) diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index d856339aa9ee..c37c4941b528 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -237,7 +237,7 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) void {{proxy_name}}::{{method.mojom_name}}IPC( std::vector::const_iterator data, size_t dataSize, - [[maybe_unused]] const std::vector &fds) + [[maybe_unused]] const std::vector &fds) { {%- for param in method.parameters %} {{param|name}} {{param.mojom_name}}; diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl index ce396c183d0c..c308dd10c7e5 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl @@ -64,7 +64,7 @@ private: void {{method.mojom_name}}IPC( std::vector::const_iterator data, size_t dataSize, - const std::vector &fds); + const std::vector &fds); {% endfor %} /* Helper class to invoke async functions in another thread. */ diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl index ebcd2aaaafae..bac826a74c2d 100644 --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl @@ -54,7 +54,7 @@ {%- for param in params %} std::vector {{param.mojom_name}}Buf; {%- if param|has_fd %} - std::vector {{param.mojom_name}}Fds; + std::vector {{param.mojom_name}}Fds; std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) = {%- else %} std::tie({{param.mojom_name}}Buf, std::ignore) = diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl index b8ef8e7b974e..77bae36fe6b7 100644 --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl @@ -40,7 +40,7 @@ retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); {%- elif field|is_fd %} std::vector {{field.mojom_name}}; - std::vector {{field.mojom_name}}Fds; + std::vector {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end()); @@ -58,7 +58,7 @@ {%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %} std::vector {{field.mojom_name}}; {%- if field|has_fd %} - std::vector {{field.mojom_name}}Fds; + std::vector {{field.mojom_name}}Fds; std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) = {%- else %} std::tie({{field.mojom_name}}, std::ignore) = @@ -177,7 +177,7 @@ # \a struct. #} {%- macro serializer(struct, namespace) %} - static std::tuple, std::vector> + static std::tuple, std::vector> serialize(const {{struct|name_full}} &data, {%- if struct|needs_control_serializer %} ControlSerializer *cs) @@ -187,7 +187,7 @@ { std::vector retData; {%- if struct|has_fd %} - std::vector retFds; + std::vector retFds; {%- endif %} {%- for field in struct.fields %} {{serializer_field(field, namespace, loop)}} @@ -210,7 +210,7 @@ {%- macro deserializer_fd(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector &data, - std::vector &fds, + std::vector &fds, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -224,8 +224,8 @@ static {{struct|name_full}} deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - std::vector::const_iterator fdsBegin, - std::vector::const_iterator fdsEnd, + std::vector::const_iterator fdsBegin, + std::vector::const_iterator fdsEnd, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -234,7 +234,7 @@ { {{struct|name_full}} ret; std::vector::const_iterator m = dataBegin; - std::vector::const_iterator n = fdsBegin; + std::vector::const_iterator n = fdsBegin; size_t dataSize = std::distance(dataBegin, dataEnd); [[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd); @@ -255,7 +255,7 @@ {%- macro deserializer_fd_simple(struct, namespace) %} static {{struct|name_full}} deserialize(std::vector &data, - [[maybe_unused]] std::vector &fds, + [[maybe_unused]] std::vector &fds, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); @@ -264,8 +264,8 @@ static {{struct|name_full}} deserialize(std::vector::const_iterator dataBegin, std::vector::const_iterator dataEnd, - [[maybe_unused]] std::vector::const_iterator fdsBegin, - [[maybe_unused]] std::vector::const_iterator fdsEnd, + [[maybe_unused]] std::vector::const_iterator fdsBegin, + [[maybe_unused]] std::vector::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs); diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py index c609f4e5c062..753bfc734e56 100644 --- a/utils/ipc/generators/mojom_libcamera_generator.py +++ b/utils/ipc/generators/mojom_libcamera_generator.py @@ -77,7 +77,7 @@ def GetDefaultValue(element): if mojom.IsEnumKind(element.kind): return f'static_cast<{element.kind.mojom_name}>(0)' if isinstance(element.kind, mojom.Struct) and \ - element.kind.mojom_name == 'FileDescriptor': + element.kind.mojom_name == 'SharedFD': return '-1' return '' @@ -140,7 +140,7 @@ def HasFd(element): types = GetAllTypes(element) else: types = GetAllTypes(element.kind) - return "FileDescriptor" in types or (attrs is not None and "hasFd" in attrs) + return "SharedFD" in types or (attrs is not None and "hasFd" in attrs) def WithDefaultValues(element): return [x for x in element if HasDefaultValue(x)] @@ -221,7 +221,7 @@ def IsEnum(element): return mojom.IsEnumKind(element.kind) def IsFd(element): - return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "FileDescriptor" + return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD" def IsMap(element): return mojom.IsMapKind(element.kind)