[{"id":3269,"web_url":"https://patchwork.libcamera.org/comment/3269/","msgid":"<20191217020459.GO4856@pendragon.ideasonboard.com>","date":"2019-12-17T02:04:59","subject":"Re: [libcamera-devel] [PATCH v2 2/3] libcamera: Add FileDescriptor\n\tto help pass numerical fds around","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Tue, Dec 17, 2019 at 02:51:05AM +0100, Niklas Söderlund wrote:\n> Add a helper to make it easier to pass file descriptors around. The\n> helper class duplicates the fd which decouples it from the original fd\n> which could be closed by its owner while the new FileDescriptor remains\n> valid.\n> \n> Signed-off-by: Niklas Söderlund \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/file_descriptor.h | 33 ++++++\n> include/libcamera/meson.build | 1 +\n> src/libcamera/file_descriptor.cpp | 164 ++++++++++++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 199 insertions(+)\n> create mode 100644 include/libcamera/file_descriptor.h\n> create mode 100644 src/libcamera/file_descriptor.cpp\n> \n> diff --git a/include/libcamera/file_descriptor.h b/include/libcamera/file_descriptor.h\n> new file mode 100644\n> index 0000000000000000..e05f111d128e0ab1\n> --- /dev/null\n> +++ b/include/libcamera/file_descriptor.h\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * file_descriptor.h - File descriptor wrapper\n> + */\n> +#ifndef __LIBCAMERA_FILE_DESCRIPTOR_H__\n> +#define __LIBCAMERA_FILE_DESCRIPTOR_H__\n> +\n> +namespace libcamera {\n> +\n> +class FileDescriptor final\n> +{\n> +public:\n> +\texplicit FileDescriptor(int fd = -1);\n> +\texplicit FileDescriptor(const FileDescriptor &other);\n> +\texplicit FileDescriptor(FileDescriptor &&other);\n> +\t~FileDescriptor();\n> +\n> +\tFileDescriptor &operator=(const FileDescriptor &other);\n> +\tFileDescriptor &operator=(FileDescriptor &&other);\n> +\n> +\tint fd() const { return fd_; }\n> +\n> +private:\n> +\tvoid duplicate(int fd);\n> +\n> +\tint fd_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_FILE_DESCRIPTOR_H__ */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 99abf06099407c1f..543e6773cc5158a0 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -6,6 +6,7 @@ libcamera_api = files([\n> 'controls.h',\n> 'event_dispatcher.h',\n> 'event_notifier.h',\n> + 'file_descriptor.h',\n> 'geometry.h',\n> 'logging.h',\n> 'object.h',\n> diff --git a/src/libcamera/file_descriptor.cpp b/src/libcamera/file_descriptor.cpp\n> new file mode 100644\n> index 0000000000000000..71d1cf27be79b288\n> --- /dev/null\n> +++ b/src/libcamera/file_descriptor.cpp\n> @@ -0,0 +1,164 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * file_descriptor.cpp - File descriptor wrapper\n> + */\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file file_descriptor.h\n> + * \\brief File descriptor wrapper\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(FileDescriptor)\n> +\n> +/**\n> + * \\class FileDescriptor\n> + * \\brief RAII-style wrapper for file descriptors\n> + *\n> + * The FileDescriptor class wraps a file descriptor (expressed as a signed\n> + * integer) to provide an RAII-style mechanism for owning the file descriptor.\n> + * When constructed, the FileDescriptor instance duplicates a given file\n> + * descriptor and takes ownership of the duplicate as a resource. The copy\n> + * constructor and assignment operator duplicate the file descriptor, while the\n> + * move versions of those methods move the resource and make the original\n> + * FileDescriptor invalid. When the FileDescriptor is deleted, it closes the\n> + * file descriptor it owns, if any.\n> + */\n> +\n> +/**\n> + * \\brief Create a FileDescriptor wrapping a copy of a given \\a fd\n> + * \\param[in] fd File descriptor\n> + *\n> + * Construct a FileDescriptor from a numerical file descriptor duplicates the\n> + * \\a fd and takes ownership of the copy. The original \\a fd is left untouched,\n> + * and the caller is responsible for closing it when appropriate. The duplicated\n> + * file descriptor will be closed automatically when the FileDescriptor instance\n> + * is destroyed.\n> + */\n> +FileDescriptor::FileDescriptor(int fd)\n> +\t: fd_(-1)\n> +{\n> +\tduplicate(fd);\n> +}\n> +\n> +/**\n> + * \\brief Copy constructor, create a FileDescriptor from a copy of \\a other\n> + * \\param[in] other The other FileDescriptor\n> + *\n> + * Construct a FileDescriptor from another FileDescriptor duplicates the\n> + * wrapped numerical file descriptor and takes ownership of the copy. The\n> + * original FileDescriptor is left untouched, and the caller is responsible for\n> + * closing it when appropriate. The duplicated file descriptor will be closed\n> + * automatically when this FileDescriptor instance is destroyed.\n> + */\n> +FileDescriptor::FileDescriptor(const FileDescriptor &other)\n> +\t: FileDescriptor(other.fd_)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Move constructor, create a FileDescriptor by taking over \\a other\n> + * \\param[in] other The other FileDescriptor\n> + *\n> + * Construct a FileDescriptor by taking over a wrapped numerical file\n> + * descriptor and taking ownership of it. The \\a other FileDescriptor is\n> + * invalidated and set to -1. The taken over file descriptor will be closed\n> + * automatically when this FileDescriptor instance is destroyed.\n> + */\n> +FileDescriptor::FileDescriptor(FileDescriptor &&other)\n> +\t: fd_(utils::exchange(other.fd_, -1))\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Destroy the managed file descriptor\n> + *\n> + * If the managed file descriptor, as returned by fd(), is not equal to -1, the\n> + * file descriptor is closed.\n> + */\n> +FileDescriptor::~FileDescriptor()\n> +{\n> +\tif (fd_ != -1)\n> +\t\tclose(fd_);\n> +}\n> +\n> +/**\n> + * \\brief Copy assignment operator, replace the wrapped file descriptor with a\n> + * duplicate from \\a other\n> + * \\param[in] other The other FileDescriptor\n> + *\n> + * Close the wrapped file descriptor (if any) and duplicate the file descriptor\n> + * from \\a other. The \\a other FileDescriptor is left untouched, and the caller\n> + * is responsible for destroying it when appropriate. The duplicated file\n> + * descriptor will be closed automatically when the FileDescriptor instance is\n\ns/the FileDescriptor/this FileDescriptor/\n\n> + * destroyed.\n> + *\n> + * \\return A reference to this FileDescriptor\n> + */\n> +FileDescriptor &FileDescriptor::operator=(const FileDescriptor &other)\n> +{\n> +\tduplicate(other.fd_);\n> +\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\brief Move assignment operator, replace the wrapped file descriptor by\n> + * taking over \\a other\n> + * \\param[in] other The other FileDescriptor\n> + *\n> + * Close the wrapped file descriptor (if any) and take over the file descriptor\n> + * from \\a other. The \\a other FileDescriptor is invalidated and set to -1. The\n> + * taken over file descriptor will be closed automatically when the\n\ns/the$/this/\n\n> + * FileDescriptor instance is destroyed.\n> + *\n> + * \\return A reference to this FileDescriptor\n> + */\n> +FileDescriptor &FileDescriptor::operator=(FileDescriptor &&other)\n> +{\n> +\tif (fd_ != -1)\n> +\t\tclose(fd_);\n> +\n> +\tfd_ = utils::exchange(other.fd_, -1);\n> +\n> +\treturn *this;\n> +}\n> +\n> +/**\n> + * \\fn FileDescriptor::fd()\n> + * \\brief Retrieve the numerical file descriptor\n> + * \\return The numerical file descriptor, which may be -1 if the FileDescriptor\n> + * instance doesn't own a file descriptor\n> + */\n> +\n> +void FileDescriptor::duplicate(int fd)\n> +{\n> +\tif (fd_ != -1)\n> +\t\tclose(fd_);\n\n\t\tfd_ = -1;\n\t}\n\n> +\n> +\tif (fd < 0) {\n> +\t\tfd_ = -1;\n\nand drop this line.\n\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Failing to dup() a fd should not happen and is fatal. */\n> +\tfd_ = dup(fd);\n> +\tif (fd_ == -1) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(FileDescriptor, Fatal)\n> +\t\t\t<< \"Failed to dup() fd: \" << strerror(-ret);\n> +\t}\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index c4f965bd7413b37e..722c5bc15afe52ef 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -14,6 +14,7 @@ libcamera_sources = files([\n> 'event_dispatcher.cpp',\n> 'event_dispatcher_poll.cpp',\n> 'event_notifier.cpp',\n> + 'file_descriptor.cpp',\n> 'formats.cpp',\n> 'geometry.cpp',\n> 'ipa_context_wrapper.cpp',","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 55297601E7\n\tfor ;\n\tTue, 17 Dec 2019 03:05:10 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C88CE9C5;\n\tTue, 17 Dec 2019 03:05:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1576548309;\n\tbh=oJ+i1OnnfrvpNWGNYgSc+2YZjBF35b9G7Av9aoYoIyc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vBXo6K3T1c0nN/povmwWtb5GYPaxxN3wKVqjTAFfcCRonS5HqiQmYST08y+cz5G6k\n\tamSSqV0ODlsAyC4wfRGFT9xF0Z4QZiYoTUm+eLMxJz3WXyKt8Fb/a/XeKqIHDljRAp\n\t8BuWdtSoQJUbMnswAWR4U//EJpz0u36wkhnjA1/U=","Date":"Tue, 17 Dec 2019 04:04:59 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191217020459.GO4856@pendragon.ideasonboard.com>","References":"<20191217015106.1175515-1-niklas.soderlund@ragnatech.se>\n\t<20191217015106.1175515-3-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191217015106.1175515-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 2/3] libcamera: Add FileDescriptor\n\tto help pass numerical fds around","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 17 Dec 2019 02:05:10 -0000"}}]