From patchwork Tue Nov 30 03:37:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14856 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 E7CD2BF415 for ; Tue, 30 Nov 2021 03:38:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 351E1605B6; Tue, 30 Nov 2021 04:38:51 +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="hpcBQrOE"; 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 BB8A96059E for ; Tue, 30 Nov 2021 04:38:48 +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 51A9111C5 for ; Tue, 30 Nov 2021 04:38:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243528; bh=mHTkR1XuociDJ7aEHQY71W9VUmG/4yt6XfsRxcbf8DM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hpcBQrOEH+OvYNvsd2eyHZNZZj4VxTq/xQPPd+pCvMGTcmEmaNd+7NiS+Cv8gHeqG IVsp4O+5ZykV51YGsZBTxNz/9ceWpiBX6dmdXh5SkH2O16RtRZ07JyBNWBvfblC6rS kJILOFkL49UCfBJ5xANcRUQQXbeI9edc/Zg67Z6s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:37:59 +0200 Message-Id: <20211130033820.18235-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 01/22] 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 Tue Nov 30 03:38:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14857 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 65719BF415 for ; Tue, 30 Nov 2021 03:38:55 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2C4A7605C9; Tue, 30 Nov 2021 04:38:54 +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="rkhsVH9q"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1FAE16059E for ; Tue, 30 Nov 2021 04:38:49 +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 B8B3D8F0 for ; Tue, 30 Nov 2021 04:38:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243528; bh=L3qh8xYNoHKpcZfbvlBIve0uOhnY/bQGfPZMcjqLzrU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=rkhsVH9qkPSO9I3OxJJxWOShwb1IcuS9gWM5hyKgh6GpSvf33wxIh/yJOCF6wT1O+ nGe8VN8nMjzBcvqPwURssMlma5yV2uS6eJNvy012PcTKnXsO2gEgqZnrnjkgapjIoS ojtV1J6LwxsBKfXRYGGsDgZTpCtvwAQLjK59lD5s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:00 +0200 Message-Id: <20211130033820.18235-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 02/22] 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 ddedc78c0c10..b763110e74a6 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 e31fa3b23859..d82cb30f0e77 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 Tue Nov 30 03:38:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14858 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 7990EC324F for ; Tue, 30 Nov 2021 03:38:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BAFAC605B8; Tue, 30 Nov 2021 04:38:54 +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="U747GOlU"; 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 8D9916059E for ; Tue, 30 Nov 2021 04:38:49 +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 2A95911C5 for ; Tue, 30 Nov 2021 04:38:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243529; bh=Bnq1Q2GUIRzEiJVRrp3q18meNV3XOjCJJKsNgmZN26M=; h=From:To:Subject:Date:In-Reply-To:References:From; b=U747GOlUHelxVYnPe55cvkYNGB550EnM5dubi9aDQz4Mn1QVJyxPnvV4FjMZQQ7I1 H5BsSKDwkS60So7/SndizCvGA+zCjPKurx5JXlrr9ZE8SfylZWfGSTdUXB0+joU5X3 k2qAjaEJoBGqixyWywEz86NbAYIC4eI0PjF5wlAY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:01 +0200 Message-Id: <20211130033820.18235-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 03/22] libcamera: base: file_descriptor: Move inode() function to frame_buffer.cpp 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. As it's only used in the FrameBuffer implementation, move it to frame_buffer.cpp as a static function. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- Changes since v3: - Move the inode function to frame_buffer.cpp --- include/libcamera/base/file.h | 2 ++ include/libcamera/base/file_descriptor.h | 3 --- src/libcamera/base/file.cpp | 1 + src/libcamera/base/file_descriptor.cpp | 25 --------------------- src/libcamera/framebuffer.cpp | 28 ++++++++++++++++++++++-- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h index 55e8edd934d4..9b62871b8230 100644 --- a/include/libcamera/base/file.h +++ b/include/libcamera/base/file.h @@ -20,6 +20,8 @@ namespace libcamera { +class FileDescriptor; + class File { public: 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..a04dfacdc102 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -14,6 +14,7 @@ #include #include +#include #include /** 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..b1635f49b68a 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -8,6 +8,9 @@ #include #include "libcamera/internal/framebuffer.h" +#include + +#include #include /** @@ -207,6 +210,27 @@ FrameBuffer::Private::Private() * \brief The plane length in bytes */ +namespace { + +ino_t fileDescriptorInode(const FileDescriptor &fd) +{ + if (!fd.isValid()) + return 0; + + struct stat st; + int ret = fstat(fd.fd(), &st); + if (ret < 0) { + ret = -errno; + LOG(Buffer, Fatal) + << "Failed to fstat() fd: " << strerror(-ret); + return 0; + } + + return st.st_ino; +} + +} /* namespace */ + /** * \brief Construct a FrameBuffer with an array of planes * \param[in] planes The frame memory planes @@ -236,8 +260,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 = fileDescriptorInode(planes_[0].fd); + if (fileDescriptorInode(plane.fd) != inode) { isContiguous = false; break; } From patchwork Tue Nov 30 03:38:02 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14859 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 46E0ABF415 for ; Tue, 30 Nov 2021 03:38:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A6E3C60717; Tue, 30 Nov 2021 04:38:58 +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="EIDusvJE"; 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 3FCE3605BA for ; Tue, 30 Nov 2021 04:38:50 +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 A43028F0; Tue, 30 Nov 2021 04:38:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243529; bh=HilQ8jl7CA5hVx3Y5PCHmfR/5wqX0HIv0tjK5itDCxQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EIDusvJE4/EQ7Fc7UcXtHF7F87bia+WVsgzPULLSg2Vlf3vGnWEWs8jpygY3JvJk9 8swGXOni+SkMNHHG3HZHiIz+6PtpiZ4ZJ9o7gFK9lqTFvjkAuMXdXIFWeejtaco3Qc oOhFXtldS350Pzys2RH2PuLh1grosAXMyE7jEQTM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:02 +0200 Message-Id: <20211130033820.18235-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 04/22] 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 Tue Nov 30 03:38:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14860 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 5D1B8C324F for ; Tue, 30 Nov 2021 03:39:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C28FE605C8; Tue, 30 Nov 2021 04:38:59 +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="n7kLiFvk"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 801C8605BC for ; Tue, 30 Nov 2021 04:38:50 +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 1F5CF11C5 for ; Tue, 30 Nov 2021 04:38:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243530; bh=gsMLigkFxA6J8JCH8Vhc7pqq5yQV0AU1tWI3szv0Ox8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=n7kLiFvkazB5Ei/jFRos210hRQp2ui9Fmye3OF3AV++qCyyRZ5UVGgAITeY38mjxq eU1PcYCDvs10v6UEapOTZrjrffVoE5Y2bO8LepRxMU51JiSfP2lV6DQjP/tBuVSFrJ qdQsXA/jCu9JAzDl+v8RxXVdD4RE2aDpdtrtgGrg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:03 +0200 Message-Id: <20211130033820.18235-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 05/22] 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: Jacopo Mondi Reviewed-by: Hirokazu Honda --- Changes since v3: - Use override where necessary - Make createFd() private --- 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..eb3b591fec28 --- /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() override + { + return createFd(); + } + + int run() override + { + /* 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() override + { + if (fd_ > 0) + close(fd_); + } + +private: + 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; + } + + 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 Tue Nov 30 03:38:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14861 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 535CEBF415 for ; Tue, 30 Nov 2021 03:39:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DC71F605C3; Tue, 30 Nov 2021 04:39:00 +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="Hbll/GVc"; 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 F1709605C1 for ; Tue, 30 Nov 2021 04:38:50 +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 7E0FB8F0; Tue, 30 Nov 2021 04:38:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243530; bh=AgBMBdKWljtF8MTtTz0SyFFV23uh+ZB8kLA+21V30ws=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Hbll/GVcD2N0hFyu1yL0TNT0Lr3cK0dyg749EBrzru23r2ftCDQy85Uy8T7cw9Pfe dQL1Rj2z/WhwXYxwXuazSPdqNCsxXjACmFesgwT70/AwILfxiMz9UImn1xZu5bDfNV wfJm0CP0crwBCZ05zT20LF4XuyWFp9L7PbP0alTA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:04 +0200 Message-Id: <20211130033820.18235-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 06/22] 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 Tue Nov 30 03:38:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14863 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 7A0D8C3250 for ; Tue, 30 Nov 2021 03:39:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E876B606FD; Tue, 30 Nov 2021 04:39:02 +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="s+F9Ximf"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 60319605C2 for ; Tue, 30 Nov 2021 04:38:51 +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 ECEC711C5 for ; Tue, 30 Nov 2021 04:38:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243531; bh=6VQC/oNlDxzmvYqn6LpHYeUHO8aITJVndH01oUYvOAM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=s+F9Ximf9tCdS7PI8HCrZiAf80zlVL11SFlx/rnbMVyOx8oigGVH1ELuRtlyV3qox UwKT/yAWGrySlnswXN8j3lQdc/G/eG2VVVvwL3ig2m+Els7RbWx3AjOEOzwaOyuyaZ 9xoT4K0LYa+YGNyt6ayKqDN6xcGE4hrM5CNe3RZo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:05 +0200 Message-Id: <20211130033820.18235-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 07/22] 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: Hirokazu Honda 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 Tue Nov 30 03:38:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14862 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 4E725C324F for ; Tue, 30 Nov 2021 03:39:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DAF3B605C8; Tue, 30 Nov 2021 04:39:01 +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="dIWiFPW7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C02B3605C4 for ; Tue, 30 Nov 2021 04:38:51 +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 580118F0; Tue, 30 Nov 2021 04:38:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243531; bh=ADcLrnJXMzRoiajlsZF9dxsL0AsNDStcNCtprfCjW6I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dIWiFPW7pd+pEsJ93XI3baABrZvjYie3PB7/o2/AQe3nRDfbunvw31qa5npMUsMnZ YZQmlAKjdT2epDXh5WRLMxUhYq4NKhYJFRFQ7Em3q1dkFf13pu7hB16BqNsPkyAJd7 azYO2hnevso7gsV1nAmO7J7VoFvWAB/rW/sG7IBE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:06 +0200 Message-Id: <20211130033820.18235-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 08/22] 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 Tue Nov 30 03:38:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14865 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 162F6C324F for ; Tue, 30 Nov 2021 03:39:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id ADCC0605CB; Tue, 30 Nov 2021 04:39:08 +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="sTNNBZOP"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 635E36059E for ; Tue, 30 Nov 2021 04:38:52 +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 E133811C5; Tue, 30 Nov 2021 04:38:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243532; bh=vElHEE1F82SJUGoJvqv0YsLh86mBBcXrpD58wJKYuCY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sTNNBZOPEhFT4hM2d/t17RCi9iM35VlRpuh1xzhdvE8WcvFzrTI/bKtHuAUW6nzE9 PkVJ+mD2T/lP2p6HOTqgRFb2mXcIufw3qRJLEmANTSr/dJcXl8/++0FjT88KSZfsqU WNANz1qZmM8GUdGO+7taBvKRcBl8X+iFEwdh+nk4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:07 +0200 Message-Id: <20211130033820.18235-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 09/22] 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 9b62871b8230..dfb8dd0f19af 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(); @@ -75,7 +76,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 a04dfacdc102..cab6e714fb07 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 Tue Nov 30 03:38:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14864 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 5373BBF415 for ; Tue, 30 Nov 2021 03:39:08 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F0DFE605C0; Tue, 30 Nov 2021 04:39:07 +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="lB/r2jat"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 05402605B7 for ; Tue, 30 Nov 2021 04:38:53 +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 82DEF11FB; Tue, 30 Nov 2021 04:38:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243532; bh=GrM0K1sdUmF0a281uXKr6sIa76N/CCcYSkKarAvQ33w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lB/r2jatZ/yVHszbcj5yTPZtpbCZNCkirZDNWN1o3qSK5qUNr5Y0gNLFVNLahhNN0 YCw/xbdxeg8HSdfsM2NSOls+2DTWwOutZx/q+X4JkS5B58i8U4aWYmxntR3YWhNmzd Wrv6h4UhVirxOTKuGGazPG2KcCGUQ02zsPY53O+Y= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:08 +0200 Message-Id: <20211130033820.18235-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 10/22] 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 Reviewed-by: Jacopo Mondi --- 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 Tue Nov 30 03:38:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14867 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 D2F44C3250 for ; Tue, 30 Nov 2021 03:39:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6375F605C9; Tue, 30 Nov 2021 04:39:10 +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="VLzOLZC+"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C671605C5 for ; Tue, 30 Nov 2021 04:38:53 +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 F15658F0 for ; Tue, 30 Nov 2021 04:38:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243533; bh=HRaDc/H3d+k8FtCEKM4DP3DYdnS5/MmVHIPIqXvhsnQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=VLzOLZC+LQ3y6DpZDamziAObYPkUC6ve4QPMM7helm+k7zXBfFzUYAn8gCGsCja7V pS6oCdT5UThELYE2R9U8DUs44bW2PVF5AZLYBnN00DIsF6bwJqle/ZkBvxQCN3rQ+T W+xvxOCH1Lx1lADVNuNo83GRzWFcWohixxToj2c0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:09 +0200 Message-Id: <20211130033820.18235-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 11/22] libcamera: ipc_unixsocket: Fix file descriptor leak 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 file descriptor created for the remote side of the socket is passed to the forked process, but never closed. Fix the leak. The fix can be tested by running the unixsocket_ipc unit test under valgrind with `valgrind --track-fds=yes ./test/ipc/unixsocket_ipc`. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- src/libcamera/ipc_pipe_unixsocket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp index 65277500ff42..3ef907090131 100644 --- a/src/libcamera/ipc_pipe_unixsocket.cpp +++ b/src/libcamera/ipc_pipe_unixsocket.cpp @@ -38,7 +38,7 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath, } socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead); args.push_back(std::to_string(fd.get())); - fds.push_back(fd.release()); + fds.push_back(fd.get()); proc_ = std::make_unique(); int ret = proc_->start(ipaProxyWorkerPath, args, fds); From patchwork Tue Nov 30 03:38:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14866 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 D0385BF415 for ; Tue, 30 Nov 2021 03:39:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8243D605C2; Tue, 30 Nov 2021 04:39:09 +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="hoD2yPQs"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 01B4E605A3 for ; Tue, 30 Nov 2021 04:38:54 +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 847028F0; Tue, 30 Nov 2021 04:38:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243533; bh=87NwiFNikiBaMpO8XkCjgfTu6j+Wt/IonCSqtM951Vc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hoD2yPQs/wFUJhk4nqLg7WaJCwbnKttOl0cUEIcBrT4lwCwuofu37q4Ih0VjDy8+m MwduXHzjCLnAeLAMXPVF2QK6n+ZNki7EQ2rZju8fbMWyD6sZD5u29wvChEsfZE3lNC yqEZalBoCbQNIATzPnRdWSQBjJtIX3pHo6NkQGrE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:10 +0200 Message-Id: <20211130033820.18235-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 12/22] 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 Tue Nov 30 03:38:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14868 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 7455CC324F for ; Tue, 30 Nov 2021 03:39:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2082B605CA; Tue, 30 Nov 2021 04:39:11 +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="QebHCXQL"; 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 9540E605CB for ; Tue, 30 Nov 2021 04:38:54 +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 2B1C78F0; Tue, 30 Nov 2021 04:38:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243534; bh=oNBjFN0xuIBQza74WRtXsPByAO9ei0dL+JgjmbSvjbw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QebHCXQLIUj9ZerJs4EGXeTp9XUaE61LK8vFcGibW8x/Btcx6VhRcPjA9LYPXiCro S+zSNheXAMJDJyvaW3VoQdNrFYYFSIXCzlKZdai6Ye2loXB7AjVJQCwtSBULWQzBR3 S+QobJqDxetO6cIkC+4QLH4ByZgHjRQDvcMVHAxo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:11 +0200 Message-Id: <20211130033820.18235-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 13/22] 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 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 Tue Nov 30 03:38:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14869 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 19C7EBF415 for ; Tue, 30 Nov 2021 03:39:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BFA80605C7; Tue, 30 Nov 2021 04:39:11 +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="Yl7r2BRB"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 10673605CF for ; Tue, 30 Nov 2021 04:38:55 +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 976B611C5; Tue, 30 Nov 2021 04:38:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243534; bh=H24AFrqDzwI42qQ6RLWntdF32rr49kklWsD1riSF05M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Yl7r2BRBGid5JW/59iGgPnrvD8EEa/TnZWIagZAN9su2vDpmQZSCWHLN9MdCHUJzq mwQsf0OrDDLwGNIiPugKQ/aKKkonFdhmUxhxGIfv9HZPflnoSEfqNvesMHyvysTu2O oSKx6ru61HnTRRb6AJMOLT2J03zO4Ey0ISh/BseU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:12 +0200 Message-Id: <20211130033820.18235-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 14/22] 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 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 Tue Nov 30 03:38:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14871 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 68267C3252 for ; Tue, 30 Nov 2021 03:39:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1521E605C9; Tue, 30 Nov 2021 04:39:13 +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="r20bkne5"; 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 867B3605D3 for ; Tue, 30 Nov 2021 04:38:55 +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 132D88F0; Tue, 30 Nov 2021 04:38:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243535; bh=xt3Pu3F2X8AkvXsg1TxYcKeyvtD5XDYdaoibapGV62Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=r20bkne5n9n3e0l+MGlEMkbCi6JpCYulGTRGVVKpbczFW8qfZ3kQj8fX36e8xR1AK rUEWxyUL6BMd+ypWGk2XLeDQ9Zey/Uj0sY29dJTYyiTFMetW+VtgDphf2+pCTZD69F pK8Av7gqDt8763MCi/R/Zn5fL0QGO/ZCLlDJuPGo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:13 +0200 Message-Id: <20211130033820.18235-16-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 15/22] 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 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 Tue Nov 30 03:38:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14870 X-Patchwork-Delegate: laurent.pinchart@ideasonboard.com 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 BC66EC3251 for ; Tue, 30 Nov 2021 03:39:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6397D605C2; Tue, 30 Nov 2021 04:39:12 +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="bHsiojhX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 21AB5605BA for ; Tue, 30 Nov 2021 04:38:56 +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 B05C211FB for ; Tue, 30 Nov 2021 04:38:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243535; bh=SeyzkNKxBvNqGt6W5rdjec51yqCbeLJdUCINRKHg0p4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=bHsiojhXi2dqgNmbHtzO1WAatEXC7o48XxScwkT80UP1piDQoQFc/o9So9BiN0gXY CkVh7kJl/3WVHmHngkHKOpph8IUrvvkqLKAT0HoK4GKh/0NsczJ8bDJnY8CexnyUj0 ylyD3pFotNpzWG1HmtL6jQ9kgr7/OltEap9USmfY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:14 +0200 Message-Id: <20211130033820.18235-17-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 16/22] libcamera: v4l2_videodevice: Pass SharedFD to open() 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 V4L2VideoDevice::open() function that takes an open file handle duplicates it internally, and leaves the original handle untouched. This is documented but not enforced through language constructs. Fix it by passing a SharedFD to the function. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Jacopo Mondi --- include/libcamera/internal/v4l2_videodevice.h | 2 +- src/libcamera/v4l2_videodevice.cpp | 22 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 80b665771782..9f556f99587c 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -184,7 +184,7 @@ public: ~V4L2VideoDevice(); int open(); - int open(int handle, enum v4l2_buf_type type); + int open(FileDescriptor handle, enum v4l2_buf_type type); void close(); const char *driverName() const { return caps_.driver(); } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 3966483a365f..5f1cc6e2f47f 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -612,16 +612,14 @@ int V4L2VideoDevice::open() * its type from the capabilities. This can be used to force a given device type * for memory-to-memory devices. * - * The file descriptor \a handle is duplicated, and the caller is responsible - * for closing the \a handle when it has no further use for it. The close() - * function will close the duplicated file descriptor, leaving \a handle - * untouched. + * The file descriptor \a handle is duplicated, no reference to the original + * handle is kept. * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) +int V4L2VideoDevice::open(FileDescriptor handle, enum v4l2_buf_type type) { - UniqueFD newFd(dup(handle)); + UniqueFD newFd = handle.dup(); if (!newFd.isValid()) { LOG(V4L2, Error) << "Failed to duplicate file handle: " << strerror(errno); @@ -1900,12 +1898,10 @@ int V4L2M2MDevice::open() /* * The output and capture V4L2VideoDevice instances use the same file - * handle for the same device node. The local file handle can be closed - * as the V4L2VideoDevice::open() retains a handle by duplicating the - * fd passed in. + * handle for the same device node. */ - UniqueFD fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), - O_RDWR | O_NONBLOCK)); + FileDescriptor 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: " @@ -1913,11 +1909,11 @@ int V4L2M2MDevice::open() return ret; } - ret = output_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_OUTPUT); + ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT); if (ret) goto err; - ret = capture_->open(fd.get(), V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE); if (ret) goto err; From patchwork Tue Nov 30 03:38:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14872 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 F07E2C3250 for ; Tue, 30 Nov 2021 03:39:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A907F60720; Tue, 30 Nov 2021 04:39:13 +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="XtFNoVUW"; 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 8B69F605B6 for ; Tue, 30 Nov 2021 04:38:56 +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 1A7368F0; Tue, 30 Nov 2021 04:38:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243536; bh=3BX2MS1kaMxk0kaqPaIUH/ZPZT0B3qL5SJ2IEKDbyws=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XtFNoVUWGZ3DmllPfsFeM+XBBBmyky4izkI+p6R4+yuNUu5wdpES/BMxbQS2lsv21 hZxYhkhIvNJwNC8qnbK5pf3xS8fVf7YbI0X/tvtx6L6pmymM/shjH86OanmBGJz8U9 OWkzVWX94uXPk10jevMLs6GhpD3Us2Ls2YGpI0GI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:15 +0200 Message-Id: <20211130033820.18235-18-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 17/22] 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 Reviewed-by: Jacopo Mondi --- 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 d82cb30f0e77..d4f248a799e5 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1382,10 +1382,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 Tue Nov 30 03:38:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14873 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 96EC4C3253 for ; Tue, 30 Nov 2021 03:39:14 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3FB57605CA; Tue, 30 Nov 2021 04:39:14 +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="CFRLrV07"; 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 0B518605BC for ; Tue, 30 Nov 2021 04:38:57 +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 8A48111C5; Tue, 30 Nov 2021 04:38:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243536; bh=lPfrrY2+CW6tNC+uPjP/b4CBUIae3kP/El/iB512rdU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CFRLrV07tt/qMYIuniPW0CbNtScwrDuQTRv4wcB5zM31tVo3Foi7bL2QVZxenfAIo vGHhUxQDQTgbUpsdh44HAfzoyDIBKjN2WCclRSzqB7UQqG3TkSv4dUmSnj8qHKJtbl SCLeO3EGj7wJt0q5h4sUyAPccAQorrLzPGW5y3dE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:16 +0200 Message-Id: <20211130033820.18235-19-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 18/22] 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 Tue Nov 30 03:38:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14876 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 59D09BF415 for ; Tue, 30 Nov 2021 03:39:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0D1C260723; Tue, 30 Nov 2021 04:39:16 +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="SKOCTEe5"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B5C53604FC for ; Tue, 30 Nov 2021 04:38:57 +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 057588F0 for ; Tue, 30 Nov 2021 04:38:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243537; bh=ovrDyv+bq/HPppuDfWeBN+nyKITsaol5d6VuU8zHUFQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=SKOCTEe5+5bxz+UTyA49qapRKNQLuEKwSWG9htf2scbbnR2/hOrUXpi4uG1wQkz8d KKcOrEE6T/C9n/9I9wYDxcJt7OU5kEM94Hfl+xhofAYCzf3tOQLwisaVXpbuA5GVYo WJfgTS2jBmJVwp/1WcW6zmgZMN29ylcld/2890nk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:17 +0200 Message-Id: <20211130033820.18235-20-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 19/22] 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 | 2 +- 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/internal/v4l2_videodevice.h | 2 +- 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 | 2 +- src/libcamera/base/file_descriptor.cpp | 266 ------------------ src/libcamera/base/meson.build | 2 +- src/libcamera/base/shared_fd.cpp | 262 +++++++++++++++++ src/libcamera/framebuffer.cpp | 8 +- src/libcamera/ipa_data_serializer.cpp | 100 +++---- src/libcamera/ipc_pipe.cpp | 4 +- .../pipeline/raspberrypi/raspberrypi.cpp | 6 +- src/libcamera/v4l2_videodevice.cpp | 12 +- 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 +- 29 files changed, 424 insertions(+), 428 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 dfb8dd0f19af..eac9f036d65b 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 { 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/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 9f556f99587c..5ba2b54643ac 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -184,7 +184,7 @@ public: ~V4L2VideoDevice(); int open(); - int open(FileDescriptor handle, enum v4l2_buf_type type); + int open(SharedFD handle, enum v4l2_buf_type type); void close(); const char *driverName() const { return caps_.driver(); } 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 cab6e714fb07..fb3e276d6ccd 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 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 b1635f49b68a..f035e16859f8 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -182,9 +182,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. @@ -212,7 +212,7 @@ FrameBuffer::Private::Private() namespace { -ino_t fileDescriptorInode(const FileDescriptor &fd) +ino_t fileDescriptorInode(const SharedFD &fd) { if (!fd.isValid()) return 0; 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 d4f248a799e5..fdcd1eca11d6 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_; @@ -1386,7 +1386,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 5f1cc6e2f47f..0d214d9edc7a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -22,8 +22,8 @@ #include #include -#include #include +#include #include #include @@ -617,7 +617,7 @@ int V4L2VideoDevice::open() * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::open(FileDescriptor handle, enum v4l2_buf_type type) +int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type) { UniqueFD newFd = handle.dup(); if (!newFd.isValid()) { @@ -1323,7 +1323,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. @@ -1352,7 +1352,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)) { @@ -1900,8 +1900,8 @@ int V4L2M2MDevice::open() * The output and capture V4L2VideoDevice instances use the same file * handle for the same device node. */ - FileDescriptor fd(syscall(SYS_openat, AT_FDCWD, deviceNode_.c_str(), - O_RDWR | O_NONBLOCK)); + SharedFD 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: " 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) From patchwork Tue Nov 30 03:38:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14874 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 308D5C324F for ; Tue, 30 Nov 2021 03:39:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D2D76605C5; Tue, 30 Nov 2021 04:39:14 +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="AwEdTUv7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 24ECC605B4 for ; Tue, 30 Nov 2021 04:38:58 +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 B358A11C5 for ; Tue, 30 Nov 2021 04:38:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243537; bh=coQj4WjDhPVcXGO++CaKss9us4nnKmA0FYDetl8KRgw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=AwEdTUv7hdshLmmKHUpMD9C5XSZuI2M2euS3g0CzDJuWgOn5tCtjWsuczHDhsOmyl 0WIIMb7xuNseSDAZQBwaeyB1e/B/To0QZBoSH/oC+HKhFXgqWpvaPT07mP7r0OW66v pds7X6JUelo99c3EjLbrrKXLjD3y113U49tsUV3c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:18 +0200 Message-Id: <20211130033820.18235-21-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 20/22] libcamera: base: shared_fd: Add comparison operators 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 == and != comparison operators between two SharedFD instances, and use them to replace manuel get() calls. Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- include/libcamera/base/shared_fd.h | 10 ++++++++++ src/libcamera/base/shared_fd.cpp | 26 ++++++++++++++++++++++++++ src/libcamera/framebuffer.cpp | 2 +- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/base/shared_fd.h index a786885ceb32..1dd51a7f046b 100644 --- a/include/libcamera/base/shared_fd.h +++ b/include/libcamera/base/shared_fd.h @@ -46,4 +46,14 @@ private: std::shared_ptr fd_; }; +static inline bool operator==(const SharedFD &lhs, const SharedFD &rhs) +{ + return lhs.get() == rhs.get(); +} + +static inline bool operator!=(const SharedFD &lhs, const SharedFD &rhs) +{ + return !(lhs == rhs); +} + } /* namespace libcamera */ diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp index 05b6892f7e19..56dc579258c7 100644 --- a/src/libcamera/base/shared_fd.cpp +++ b/src/libcamera/base/shared_fd.cpp @@ -214,6 +214,32 @@ SharedFD &SharedFD::operator=(SharedFD &&other) * instance is invalid */ +/** + * \fn bool operator==(const SharedFD &lhs, const SharedFD &rhs) + * \brief Compare the owned file descriptors of two SharedFD for equality + * \param[in] lhs The first SharedFD + * \param[in] lhs The second SharedFD + * + * Two file descriptors are considered equal if they have the same numerical + * value. File descriptors with different values that both reference the same + * file (for instance obtained using dup()) are considered not equal. + * + * \return True if the two file descriptors are equal, false otherwise + */ + +/** + * \fn bool operator!=(const SharedFD &lhs, const SharedFD &rhs) + * \brief Compare the owned file descriptors of two SharedFD for equality + * \param[in] lhs The first SharedFD + * \param[in] lhs The second SharedFD + * + * Two file descriptors are considered equal if they have the same numerical + * value. File descriptors with different values that both reference the same + * file (for instance obtained using dup()) are considered not equal. + * + * \return True if the two file descriptors are not equal, false otherwise + */ + /** * \brief Duplicate a SharedFD * diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index f035e16859f8..f313566c5561 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -258,7 +258,7 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) * Two different dmabuf file descriptors may still refer to the * same dmabuf instance. Check this using inodes. */ - if (plane.fd.fd() != planes_[0].fd.fd()) { + if (plane.fd != planes_[0].fd) { if (!inode) inode = fileDescriptorInode(planes_[0].fd); if (fileDescriptorInode(plane.fd) != inode) { From patchwork Tue Nov 30 03:38:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14875 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 BB6DAC3254 for ; Tue, 30 Nov 2021 03:39:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6B2DF60719; Tue, 30 Nov 2021 04:39:15 +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="BW5Xrn2N"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A134B605C1 for ; Tue, 30 Nov 2021 04:38:58 +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 428F68F0 for ; Tue, 30 Nov 2021 04:38:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243538; bh=FAdH9f51RJMyBse7aDEEaH7f36ny31yhQqSU9RFJofg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=BW5Xrn2NkKyZ6W7fhscuG9O81g+3khUDUeGAcgjXA8UWY8GsvlstVCDsy9UKgyXkm +WUxgVKQBktzh5J2MfDqLm+Q+4cX/SDV5Wc8EblsnaQOS4AElklLdDDugtPRw8qXyN 7jmJARc4HDWeZ8UlNMTHnOBJdUTb4oQhhmakMnwU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:19 +0200 Message-Id: <20211130033820.18235-22-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 21/22] libcamera: base: shared_fd: Rename fd() to get() 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" For consistency with UniqueFD, rename the fd() function to get(). Renaming UniqueFD::get() to fd() would have been another option, but was rejected to keep as close as possible to the std::shared_ptr<> and std::unique_ptr<> APIs. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Hirokazu Honda --- include/libcamera/base/shared_fd.h | 2 +- include/libcamera/internal/v4l2_videodevice.h | 2 +- src/cam/drm.cpp | 4 +- src/cam/image.cpp | 4 +- src/gstreamer/gstlibcameraallocator.cpp | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 2 +- src/libcamera/base/shared_fd.cpp | 4 +- src/libcamera/framebuffer.cpp | 2 +- src/libcamera/ipc_pipe.cpp | 2 +- src/libcamera/mapped_framebuffer.cpp | 4 +- .../pipeline/raspberrypi/raspberrypi.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 6 +-- src/v4l2/v4l2_camera.cpp | 2 +- test/shared-fd.cpp | 39 ++++++++++--------- 14 files changed, 39 insertions(+), 38 deletions(-) diff --git a/include/libcamera/base/shared_fd.h b/include/libcamera/base/shared_fd.h index 1dd51a7f046b..e53a8b88601e 100644 --- a/include/libcamera/base/shared_fd.h +++ b/include/libcamera/base/shared_fd.h @@ -27,7 +27,7 @@ public: SharedFD &operator=(SharedFD &&other); bool isValid() const { return fd_ != nullptr; } - int fd() const { return fd_ ? fd_->fd() : -1; } + int get() const { return fd_ ? fd_->fd() : -1; } UniqueFD dup() const; private: diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 5ba2b54643ac..9b2ec3afecff 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -140,7 +140,7 @@ private: private: struct Plane { Plane(const FrameBuffer::Plane &plane) - : fd(plane.fd.fd()), length(plane.length) + : fd(plane.fd.get()), length(plane.length) { } diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp index f25300913a7f..46e34eb546fb 100644 --- a/src/cam/drm.cpp +++ b/src/cam/drm.cpp @@ -608,12 +608,12 @@ std::unique_ptr Device::createFrameBuffer( unsigned int i = 0; for (const libcamera::FrameBuffer::Plane &plane : planes) { - int fd = plane.fd.fd(); + int fd = plane.fd.get(); uint32_t handle; auto iter = fb->planes_.find(fd); if (iter == fb->planes_.end()) { - ret = drmPrimeFDToHandle(fd_, plane.fd.fd(), &handle); + ret = drmPrimeFDToHandle(fd_, plane.fd.get(), &handle); if (ret < 0) { ret = -errno; std::cerr diff --git a/src/cam/image.cpp b/src/cam/image.cpp index 0180be78d396..fe2cc6da5a15 100644 --- a/src/cam/image.cpp +++ b/src/cam/image.cpp @@ -39,7 +39,7 @@ std::unique_ptr Image::fromFrameBuffer(const FrameBuffer *buffer, MapMode std::map mappedBuffers; for (const FrameBuffer::Plane &plane : buffer->planes()) { - const int fd = plane.fd.fd(); + const int fd = plane.fd.get(); if (mappedBuffers.find(fd) == mappedBuffers.end()) { const size_t length = lseek(fd, 0, SEEK_END); mappedBuffers[fd] = MappedBufferInfo{ nullptr, 0, length }; @@ -61,7 +61,7 @@ std::unique_ptr Image::fromFrameBuffer(const FrameBuffer *buffer, MapMode } for (const FrameBuffer::Plane &plane : buffer->planes()) { - const int fd = plane.fd.fd(); + const int fd = plane.fd.get(); auto &info = mappedBuffers[fd]; if (!info.address) { void *address = mmap(nullptr, info.mapLength, mmapFlags, diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp index cb07d6e9e87f..c740b8fc82a8 100644 --- a/src/gstreamer/gstlibcameraallocator.cpp +++ b/src/gstreamer/gstlibcameraallocator.cpp @@ -52,7 +52,7 @@ FrameWrap::FrameWrap(GstAllocator *allocator, FrameBuffer *buffer, outstandingPlanes_(0) { for (const FrameBuffer::Plane &plane : buffer->planes()) { - GstMemory *mem = gst_fd_allocator_alloc(allocator, plane.fd.fd(), + GstMemory *mem = gst_fd_allocator_alloc(allocator, plane.fd.get(), plane.offset + plane.length, GST_FD_MEMORY_FLAG_DONT_CLOSE); gst_memory_resize(mem, plane.offset, plane.length); diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index aaf629eeb3fc..0ed41385018a 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -377,7 +377,7 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, lsTableHandle_ = std::move(ipaConfig.lsTableHandle); if (lsTableHandle_.isValid()) { lsTable_ = mmap(nullptr, ipa::RPi::MaxLsGridSize, PROT_READ | PROT_WRITE, - MAP_SHARED, lsTableHandle_.fd(), 0); + MAP_SHARED, lsTableHandle_.get(), 0); if (lsTable_ == MAP_FAILED) { LOG(IPARPI, Error) << "dmaHeap mmap failure for LS table."; diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp index 56dc579258c7..0c8b93a47f85 100644 --- a/src/libcamera/base/shared_fd.cpp +++ b/src/libcamera/base/shared_fd.cpp @@ -208,7 +208,7 @@ SharedFD &SharedFD::operator=(SharedFD &&other) */ /** - * \fn SharedFD::fd() + * \fn SharedFD::get() * \brief Retrieve the numerical file descriptor * \return The numerical file descriptor, which may be -1 if the SharedFD * instance is invalid @@ -253,7 +253,7 @@ SharedFD &SharedFD::operator=(SharedFD &&other) */ UniqueFD SharedFD::dup() const { - int dupFd = ::dup(fd()); + int dupFd = ::dup(get()); if (dupFd == -1) { int ret = -errno; LOG(SharedFD, Error) diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp index f313566c5561..701212f3ae21 100644 --- a/src/libcamera/framebuffer.cpp +++ b/src/libcamera/framebuffer.cpp @@ -218,7 +218,7 @@ ino_t fileDescriptorInode(const SharedFD &fd) return 0; struct stat st; - int ret = fstat(fd.fd(), &st); + int ret = fstat(fd.get(), &st); if (ret < 0) { ret = -errno; LOG(Buffer, Fatal) diff --git a/src/libcamera/ipc_pipe.cpp b/src/libcamera/ipc_pipe.cpp index 3b47032de0a2..31a0ca09f9f0 100644 --- a/src/libcamera/ipc_pipe.cpp +++ b/src/libcamera/ipc_pipe.cpp @@ -113,7 +113,7 @@ IPCUnixSocket::Payload IPCMessage::payload() const } for (const SharedFD &fd : fds_) - payload.fds.push_back(fd.fd()); + payload.fds.push_back(fd.get()); return payload; } diff --git a/src/libcamera/mapped_framebuffer.cpp b/src/libcamera/mapped_framebuffer.cpp index 464d35fef2d2..6860069b68ca 100644 --- a/src/libcamera/mapped_framebuffer.cpp +++ b/src/libcamera/mapped_framebuffer.cpp @@ -198,7 +198,7 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) std::map mappedBuffers; for (const FrameBuffer::Plane &plane : buffer->planes()) { - const int fd = plane.fd.fd(); + const int fd = plane.fd.get(); if (mappedBuffers.find(fd) == mappedBuffers.end()) { const size_t length = lseek(fd, 0, SEEK_END); mappedBuffers[fd] = MappedBufferInfo{ nullptr, 0, length }; @@ -220,7 +220,7 @@ MappedFrameBuffer::MappedFrameBuffer(const FrameBuffer *buffer, MapFlags flags) } for (const FrameBuffer::Plane &plane : buffer->planes()) { - const int fd = plane.fd.fd(); + const int fd = plane.fd.get(); auto &info = mappedBuffers[fd]; if (!info.address) { void *address = mmap(nullptr, info.mapLength, mmapFlags, diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index fdcd1eca11d6..fa2848533b29 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1470,7 +1470,7 @@ void RPiCameraData::setIspControls(const ControlList &controls) Span s = value.data(); bcm2835_isp_lens_shading *ls = reinterpret_cast(s.data()); - ls->dmabuf = lsTable_.fd(); + ls->dmabuf = lsTable_.get(); } isp_[Isp::Input].dev()->setControls(&ctrls); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 0d214d9edc7a..b4b89e2759b9 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -282,7 +282,7 @@ bool V4L2BufferCache::Entry::operator==(const FrameBuffer &buffer) const return false; for (unsigned int i = 0; i < planes.size(); i++) - if (planes_[i].fd != planes[i].fd.fd() || + if (planes_[i].fd != planes[i].fd.get() || planes_[i].length != planes[i].length) return false; return true; @@ -1517,9 +1517,9 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { for (unsigned int p = 0; p < numV4l2Planes; ++p) - v4l2Planes[p].m.fd = planes[p].fd.fd(); + v4l2Planes[p].m.fd = planes[p].fd.get(); } else { - buf.m.fd = planes[0].fd.fd(); + buf.m.fd = planes[0].fd.get(); } } diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 464507505c79..e922b9e6aab2 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -195,7 +195,7 @@ int V4L2Camera::getBufferFd(unsigned int index) if (buffers.size() <= index) return -1; - return buffers[index]->planes()[0].fd.fd(); + return buffers[index]->planes()[0].fd.get(); } int V4L2Camera::streamOn() diff --git a/test/shared-fd.cpp b/test/shared-fd.cpp index 60e5d0aaa395..997d7be18c47 100644 --- a/test/shared-fd.cpp +++ b/test/shared-fd.cpp @@ -46,7 +46,7 @@ protected: /* Test creating empty SharedFD. */ desc1_ = new SharedFD(); - if (desc1_->fd() != -1) { + if (desc1_->get() != -1) { std::cout << "Failed fd numerical check (default constructor)" << std::endl; return TestFail; @@ -60,19 +60,19 @@ protected: * descriptor. */ desc1_ = new SharedFD(fd_); - if (desc1_->fd() == fd_) { + if (desc1_->get() == fd_) { std::cout << "Failed fd numerical check (lvalue ref constructor)" << std::endl; return TestFail; } - if (!isValidFd(fd_) || !isValidFd(desc1_->fd())) { + if (!isValidFd(fd_) || !isValidFd(desc1_->get())) { std::cout << "Failed fd validity after construction (lvalue ref constructor)" << std::endl; return TestFail; } - int fd = desc1_->fd(); + int fd = desc1_->get(); delete desc1_; desc1_ = nullptr; @@ -91,19 +91,19 @@ protected: int dupFdCopy = dupFd; desc1_ = new SharedFD(std::move(dupFd)); - if (desc1_->fd() != dupFdCopy) { + if (desc1_->get() != dupFdCopy) { std::cout << "Failed fd numerical check (rvalue ref constructor)" << std::endl; return TestFail; } - if (dupFd != -1 || !isValidFd(fd_) || !isValidFd(desc1_->fd())) { + if (dupFd != -1 || !isValidFd(fd_) || !isValidFd(desc1_->get())) { std::cout << "Failed fd validity after construction (rvalue ref constructor)" << std::endl; return TestFail; } - fd = desc1_->fd(); + fd = desc1_->get(); delete desc1_; desc1_ = nullptr; @@ -118,13 +118,14 @@ protected: desc1_ = new SharedFD(fd_); desc2_ = new SharedFD(*desc1_); - if (desc1_->fd() == fd_ || desc2_->fd() == fd_ || desc1_->fd() != desc2_->fd()) { + if (desc1_->get() == fd_ || desc2_->get() == fd_ || + desc1_->get() != desc2_->get()) { std::cout << "Failed fd numerical check (copy constructor)" << std::endl; return TestFail; } - if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { + if (!isValidFd(desc1_->get()) || !isValidFd(desc2_->get())) { std::cout << "Failed fd validity after construction (copy constructor)" << std::endl; return TestFail; @@ -133,7 +134,7 @@ protected: delete desc1_; desc1_ = nullptr; - if (!isValidFd(desc2_->fd())) { + if (!isValidFd(desc2_->get())) { std::cout << "Failed fd validity after destruction (copy constructor)" << std::endl; return TestFail; @@ -144,16 +145,16 @@ protected: /* Test creating SharedFD by taking over other SharedFD. */ desc1_ = new SharedFD(fd_); - fd = desc1_->fd(); + fd = desc1_->get(); desc2_ = new SharedFD(std::move(*desc1_)); - if (desc1_->fd() != -1 || desc2_->fd() != fd) { + if (desc1_->get() != -1 || desc2_->get() != fd) { std::cout << "Failed fd numerical check (move constructor)" << std::endl; return TestFail; } - if (!isValidFd(desc2_->fd())) { + if (!isValidFd(desc2_->get())) { std::cout << "Failed fd validity after construction (move constructor)" << std::endl; return TestFail; @@ -168,16 +169,16 @@ protected: desc1_ = new SharedFD(); desc2_ = new SharedFD(fd_); - fd = desc2_->fd(); + fd = desc2_->get(); *desc1_ = *desc2_; - if (desc1_->fd() != fd || desc2_->fd() != fd) { + if (desc1_->get() != fd || desc2_->get() != fd) { std::cout << "Failed fd numerical check (copy assignment)" << std::endl; return TestFail; } - if (!isValidFd(desc1_->fd()) || !isValidFd(desc2_->fd())) { + if (!isValidFd(desc1_->get()) || !isValidFd(desc2_->get())) { std::cout << "Failed fd validity after construction (copy assignment)" << std::endl; return TestFail; @@ -192,16 +193,16 @@ protected: desc1_ = new SharedFD(); desc2_ = new SharedFD(fd_); - fd = desc2_->fd(); + fd = desc2_->get(); *desc1_ = std::move(*desc2_); - if (desc1_->fd() != fd || desc2_->fd() != -1) { + if (desc1_->get() != fd || desc2_->get() != -1) { std::cout << "Failed fd numerical check (move assignment)" << std::endl; return TestFail; } - if (!isValidFd(desc1_->fd())) { + if (!isValidFd(desc1_->get())) { std::cout << "Failed fd validity after construction (move assignment)" << std::endl; return TestFail; From patchwork Tue Nov 30 03:38:20 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 14877 X-Patchwork-Delegate: laurent.pinchart@ideasonboard.com 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 07BDEC3255 for ; Tue, 30 Nov 2021 03:39:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A479760727; Tue, 30 Nov 2021 04:39:16 +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="YANpMEkU"; 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 082AC605D2 for ; Tue, 30 Nov 2021 04:38:59 +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 A346B11C5 for ; Tue, 30 Nov 2021 04:38:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1638243538; bh=SqUv1cjL1iMmgnUdTeHgH5AAculkGF2lt3yu8foY4iY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YANpMEkU2RhdJwT7T9CyL+da0WhSF2ltglF3I9ot3rXBb+1NvEtN9tVtSbO1P+VSp GRWuqsXKo4X/MzTP0263sW68jdAYjI7s9Zbj0E//crSQWRBJ40a64mgPRc2PLkNWnw avLphdxlUxVX/9XKgZDFWrhytHY9iYenWex5+iw8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Nov 2021 05:38:20 +0200 Message-Id: <20211130033820.18235-23-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> References: <20211130033820.18235-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 22/22] libcamera: base: unique_fd: Pass rvalue reference to constructor and reset() 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" To avoid the risk of using a file descriptor whose ownership has been transferred to a UniqueFD instance, pass an rvalue reference to the constructor taking an int, and to the reset() function. The fd argument is set to -1 upon return, making incorrect usage of the file descriptor impossible. Signed-off-by: Laurent Pinchart --- I'm not entirely sure about this patch. If desired, it will get squashed with the one introducing UniqueFD. --- include/libcamera/base/unique_fd.h | 5 +++-- src/libcamera/base/shared_fd.cpp | 2 +- src/libcamera/base/unique_fd.cpp | 10 ++++++++-- src/libcamera/ipc_unixsocket.cpp | 4 ++-- src/libcamera/pipeline/raspberrypi/dma_heaps.cpp | 15 ++++++--------- src/libcamera/process.cpp | 4 ++-- src/libcamera/v4l2_videodevice.cpp | 2 +- test/unique-fd.cpp | 10 ++++++---- 8 files changed, 29 insertions(+), 23 deletions(-) diff --git a/include/libcamera/base/unique_fd.h b/include/libcamera/base/unique_fd.h index ae4d96b75797..a5ffb1a435dc 100644 --- a/include/libcamera/base/unique_fd.h +++ b/include/libcamera/base/unique_fd.h @@ -22,9 +22,10 @@ public: { } - explicit UniqueFD(int fd) + explicit UniqueFD(int &&fd) : fd_(fd) { + fd = -1; } UniqueFD(UniqueFD &&other) @@ -50,7 +51,7 @@ public: return fd; } - void reset(int fd = -1); + void reset(int &&fd = -1); void swap(UniqueFD &other) { diff --git a/src/libcamera/base/shared_fd.cpp b/src/libcamera/base/shared_fd.cpp index 0c8b93a47f85..8aecd34038bd 100644 --- a/src/libcamera/base/shared_fd.cpp +++ b/src/libcamera/base/shared_fd.cpp @@ -260,7 +260,7 @@ UniqueFD SharedFD::dup() const << "Failed to dup() fd: " << strerror(-ret); } - return UniqueFD(dupFd); + return UniqueFD(std::move(dupFd)); } SharedFD::Descriptor::Descriptor(int fd, bool duplicate) diff --git a/src/libcamera/base/unique_fd.cpp b/src/libcamera/base/unique_fd.cpp index 83d6919cf623..7a961dbc01d0 100644 --- a/src/libcamera/base/unique_fd.cpp +++ b/src/libcamera/base/unique_fd.cpp @@ -38,9 +38,12 @@ LOG_DEFINE_CATEGORY(UniqueFD) */ /** - * \fn UniqueFD::UniqueFD(int fd) + * \fn UniqueFD::UniqueFD(int &&fd) * \brief Construct a UniqueFD that owns \a fd * \param[in] fd A file descriptor to manage + * + * Ownership of the file descriptor is transferred to the UniqueFD instance. + * Upon return \a fd is set to -1. */ /** @@ -88,11 +91,12 @@ LOG_DEFINE_CATEGORY(UniqueFD) * \param[in] fd The new file descriptor to manage * * Close the managed file descriptor, if any, and replace it with the new \a fd. + * Upon return \a fd is set to -1. * * Self-resetting (passing an \a fd already managed by this instance) is invalid * and results in undefined behaviour. */ -void UniqueFD::reset(int fd) +void UniqueFD::reset(int &&fd) { ASSERT(!isValid() || fd != fd_); @@ -100,6 +104,8 @@ void UniqueFD::reset(int fd) if (fd >= 0) close(fd); + + fd = -1; } /** diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index 1980d374cea8..32bd6533d192 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -103,8 +103,8 @@ UniqueFD IPCUnixSocket::create() } std::array socketFds{ - UniqueFD(sockets[0]), - UniqueFD(sockets[1]), + UniqueFD(std::move(sockets[0])), + UniqueFD(std::move(sockets[1])), }; if (bind(std::move(socketFds[0])) < 0) diff --git a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp index 69831dabbe75..f2a95300d187 100644 --- a/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp +++ b/src/libcamera/pipeline/raspberrypi/dma_heaps.cpp @@ -37,16 +37,13 @@ namespace RPi { DmaHeap::DmaHeap() { for (const char *name : heapNames) { - int ret = ::open(name, O_RDWR, 0); - if (ret < 0) { - ret = errno; - LOG(RPI, Debug) << "Failed to open " << name << ": " - << strerror(ret); - continue; - } + dmaHeapHandle_ = UniqueFD(open(name, O_RDWR, 0)); + if (dmaHeapHandle_.isValid()) + break; - dmaHeapHandle_ = UniqueFD(ret); - break; + int err = errno; + LOG(RPI, Debug) << "Failed to open " << name << ": " + << strerror(err); } if (!dmaHeapHandle_.isValid()) diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 0e6b4e1dde58..dc8576d934b6 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -134,8 +134,8 @@ ProcessManager::ProcessManager() LOG(Process, Fatal) << "Failed to initialize pipe for signal handling"; - pipe_[0] = UniqueFD(pipe[0]); - pipe_[1] = UniqueFD(pipe[1]); + pipe_[0] = UniqueFD(std::move(pipe[0])); + pipe_[1] = UniqueFD(std::move(pipe[1])); sigEvent_ = new EventNotifier(pipe_[0].get(), EventNotifier::Read); sigEvent_->activated.connect(this, &ProcessManager::sighandler); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index b4b89e2759b9..9d0d0077f4e4 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1396,7 +1396,7 @@ UniqueFD V4L2VideoDevice::exportDmabufFd(unsigned int index, return {}; } - return UniqueFD(expbuf.fd); + return UniqueFD(std::move(expbuf.fd)); } /** diff --git a/test/unique-fd.cpp b/test/unique-fd.cpp index eb3b591fec28..7e94e3ca450e 100644 --- a/test/unique-fd.cpp +++ b/test/unique-fd.cpp @@ -39,7 +39,8 @@ protected: } /* Test creating UniqueFD from numerical file descriptor. */ - UniqueFD fd2(fd_); + int numFd = fd_; + UniqueFD fd2(std::move(numFd)); if (!fd2.isValid() || fd2.get() != fd_) { std::cout << "Failed fd check (fd constructor)" << std::endl; @@ -113,7 +114,7 @@ protected: } /* Test release. */ - int numFd = fd2.release(); + numFd = fd2.release(); if (fd2.isValid() || fd2.get() != -1) { std::cout << "Failed fd check (release)" << std::endl; @@ -133,7 +134,7 @@ protected: } /* Test reset assignment. */ - fd.reset(numFd); + fd.reset(std::move(numFd)); if (!fd.isValid() || fd.get() != fd_) { std::cout << "Failed fd check (reset assignment)" << std::endl; @@ -168,7 +169,8 @@ protected: } { - UniqueFD fd4(fd_); + numFd = fd_; + UniqueFD fd4(std::move(numFd)); } if (isValidFd(fd_)) {