[{"id":30508,"web_url":"https://patchwork.libcamera.org/comment/30508/","msgid":"<87v80m9g9q.fsf@redhat.com>","date":"2024-07-31T07:23:13","subject":"Re: [PATCH 1/3] libcamera: base: Add MemFd helper class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nthank you for the cleanup.\n\nLaurent Pinchart writes:\n\n> libcamera creates memfds in two locations already, duplicating some\n> code. Move the code to a new MemFd helper class.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/base/memfd.h | 34 +++++++++\n> include/libcamera/base/meson.build | 1 +\n> src/libcamera/base/memfd.cpp | 112 ++++++++++++++++++++++++++++\n> src/libcamera/base/meson.build | 1 +\n> src/libcamera/dma_buf_allocator.cpp | 46 +-----------\n> src/libcamera/shared_mem_object.cpp | 21 ++----\n> 6 files changed, 157 insertions(+), 58 deletions(-)\n> create mode 100644 include/libcamera/base/memfd.h\n> create mode 100644 src/libcamera/base/memfd.cpp\n>\n> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h\n> new file mode 100644\n> index 000000000000..b0edd2de5d83\n> --- /dev/null\n> +++ b/include/libcamera/base/memfd.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas on Board Oy\n> + *\n> + * Anonymous file creation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class MemFd\n> +{\n> +public:\n> +\tenum class Seal {\n> +\t\tNone = 0,\n> +\t\tShrink = (1 << 0),\n> +\t\tGrow = (1 << 1),\n> +\t};\n> +\n> +\tusing Seals = Flags;\n> +\n> +\tstatic UniqueFD create(const char *name, std::size_t size,\n> +\t\t\t Seals seals = Seal::None);\n> +};\n> +\n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(MemFd::Seal)\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> index bace25d56b13..2a0cee317204 100644\n> --- a/include/libcamera/base/meson.build\n> +++ b/include/libcamera/base/meson.build\n> @@ -21,6 +21,7 @@ libcamera_base_private_headers = files([\n> 'event_notifier.h',\n> 'file.h',\n> 'log.h',\n> + 'memfd.h',\n> 'message.h',\n> 'mutex.h',\n> 'private.h',\n> diff --git a/src/libcamera/base/memfd.cpp b/src/libcamera/base/memfd.cpp\n> new file mode 100644\n> index 000000000000..0d50e2d64638\n> --- /dev/null\n> +++ b/src/libcamera/base/memfd.cpp\n> @@ -0,0 +1,112 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * Anonymous file creation\n> + */\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file base/memfd.h\n> + * \\brief Anonymous file creation\n> + */\n> +\n> +/* uClibc doesn't provide the file sealing API. */\n> +#ifndef __DOXYGEN__\n> +#if not HAVE_FILE_SEALS\n> +#define F_ADD_SEALS\t\t1033\n> +#define F_SEAL_SHRINK\t\t0x0002\n> +#define F_SEAL_GROW\t\t0x0004\n> +#endif\n> +#endif\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(File)\n> +\n> +/**\n> + * \\class MemFd\n> + * \\brief Helper class to created anonymous files\n\nYou probably meant s/created/create/ ?\n\n> + */\n> +\n> +/**\n> + * \\enum MemFd::Seal\n> + * \\brief Seals for the MemFd::create() function\n> + * \\var MemFd::Seal::None\n> + * \\brief No seals (used as default value)\n> + * \\var MemFd::Seal::Shrink\n> + * \\brief Prevent the memfd from shrinking\n> + * \\var MemFd::Seal::Grow\n> + * \\brief Prevent the memfd from growing\n> + */\n> +\n> +/**\n> + * \\typedef MemFd::Seals\n> + * \\brief A bitwise combination of MemFd::Seal values\n> + */\n> +\n> +/**\n> + * \\brief Create an anonymous file\n> + * \\param[in] name The file name (displayed in symbolic links in /proc/self/fd/)\n> + * \\param[in] size The file size\n> + * \\param[in] seals The file seals\n> + * \\return The descriptor of the anonymous file is creation succeeded, or an\n\ns/is/if/\n\n> + *\n> + * This function is a helper that wraps anonymous file (memfd) creation and\n> + * sets the file size and optional seals.\n> + *\n> + * invalid UniqueFD otherwise\n\nThis line should be attached to \\return above.\n\nWith the typos fixed:\n\nReviewed-by: Milan Zamazal \n\n> + */\n> +UniqueFD MemFd::create(const char *name, std::size_t size, Seals seals)\n> +{\n> +#if HAVE_MEMFD_CREATE\n> +\tint ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> +#else\n> +\tint ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> +#endif\n> +\tif (ret < 0) {\n> +\t\tret = errno;\n> +\t\tLOG(File, Error)\n> +\t\t\t<< \"Failed to allocate memfd storage for \" << name\n> +\t\t\t<< \": \" << strerror(ret);\n> +\t\treturn {};\n> +\t}\n> +\n> +\tUniqueFD memfd(ret);\n> +\n> +\tret = ftruncate(memfd.get(), size);\n> +\tif (ret < 0) {\n> +\t\tret = errno;\n> +\t\tLOG(File, Error)\n> +\t\t\t<< \"Failed to set memfd size for \" << name\n> +\t\t\t<< \": \" << strerror(ret);\n> +\t\treturn {};\n> +\t}\n> +\n> +\tif (seals) {\n> +\t\tint fileSeals = (seals & Seal::Shrink ? F_SEAL_SHRINK : 0)\n> +\t\t\t | (seals & Seal::Grow ? F_SEAL_GROW : 0);\n> +\n> +\t\tret = fcntl(memfd.get(), F_ADD_SEALS, fileSeals);\n> +\t\tif (ret < 0) {\n> +\t\t\tret = errno;\n> +\t\t\tLOG(File, Error)\n> +\t\t\t\t<< \"Failed to seal the memfd for \" << name\n> +\t\t\t\t<< \": \" << strerror(ret);\n> +\t\t\treturn {};\n> +\t\t}\n> +\t}\n> +\n> +\treturn memfd;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> index 7a7fd7e4ca87..4a228d335ba4 100644\n> --- a/src/libcamera/base/meson.build\n> +++ b/src/libcamera/base/meson.build\n> @@ -10,6 +10,7 @@ libcamera_base_sources = files([\n> 'file.cpp',\n> 'flags.cpp',\n> 'log.cpp',\n> + 'memfd.cpp',\n> 'message.cpp',\n> 'mutex.cpp',\n> 'object.cpp',\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index c06eca7d04ec..be6efb89fbb7 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -13,7 +13,6 @@\n> #include \n> #include \n> #include \n> -#include \n> #include \n> #include \n> \n> @@ -22,6 +21,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> /**\n> * \\file dma_buf_allocator.cpp\n> @@ -126,54 +126,16 @@ DmaBufAllocator::~DmaBufAllocator() = default;\n> * \\brief Check if the DmaBufAllocator instance is valid\n> * \\return True if the DmaBufAllocator is valid, false otherwise\n> */\n> -\n> -/* uClibc doesn't provide the file sealing API. */\n> -#ifndef __DOXYGEN__\n> -#if not HAVE_FILE_SEALS\n> -#define F_ADD_SEALS\t\t1033\n> -#define F_SEAL_SHRINK\t\t0x0002\n> -#endif\n> -#endif\n> -\n> UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)\n> {\n> \t/* Size must be a multiple of the page size. Round it up. */\n> \tstd::size_t pageMask = sysconf(_SC_PAGESIZE) - 1;\n> \tsize = (size + pageMask) & ~pageMask;\n> \n> -#if HAVE_MEMFD_CREATE\n> -\tint ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> -#else\n> -\tint ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> -#endif\n> -\tif (ret < 0) {\n> -\t\tret = errno;\n> -\t\tLOG(DmaBufAllocator, Error)\n> -\t\t\t<< \"Failed to allocate memfd storage for \" << name\n> -\t\t\t<< \": \" << strerror(ret);\n> -\t\treturn {};\n> -\t}\n> -\n> -\tUniqueFD memfd(ret);\n> -\n> -\tret = ftruncate(memfd.get(), size);\n> -\tif (ret < 0) {\n> -\t\tret = errno;\n> -\t\tLOG(DmaBufAllocator, Error)\n> -\t\t\t<< \"Failed to set memfd size for \" << name\n> -\t\t\t<< \": \" << strerror(ret);\n> -\t\treturn {};\n> -\t}\n> -\n> \t/* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */\n> -\tret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);\n> -\tif (ret < 0) {\n> -\t\tret = errno;\n> -\t\tLOG(DmaBufAllocator, Error)\n> -\t\t\t<< \"Failed to seal the memfd for \" << name\n> -\t\t\t<< \": \" << strerror(ret);\n> +\tUniqueFD memfd = MemFd::create(name, size, MemFd::Seal::Shrink);\n> +\tif (!memfd.isValid())\n> \t\treturn {};\n> -\t}\n> \n> \tstruct udmabuf_create create;\n> \n> @@ -182,7 +144,7 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)\n> \tcreate.offset = 0;\n> \tcreate.size = size;\n> \n> -\tret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create);\n> +\tint ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create);\n> \tif (ret < 0) {\n> \t\tret = errno;\n> \t\tLOG(DmaBufAllocator, Error)\n> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n> index 809fbdaf95de..022645e71a35 100644\n> --- a/src/libcamera/shared_mem_object.cpp\n> +++ b/src/libcamera/shared_mem_object.cpp\n> @@ -13,9 +13,9 @@\n> #include \n> #include \n> #include \n> -#include \n> #include \n> -#include \n> +\n> +#include \n> \n> /**\n> * \\file shared_mem_object.cpp\n> @@ -58,22 +58,11 @@ SharedMem::SharedMem() = default;\n> */\n> SharedMem::SharedMem(const std::string &name, std::size_t size)\n> {\n> -#if HAVE_MEMFD_CREATE\n> -\tint fd = memfd_create(name.c_str(), MFD_CLOEXEC);\n> -#else\n> -\tint fd = syscall(SYS_memfd_create, name.c_str(), MFD_CLOEXEC);\n> -#endif\n> -\tif (fd < 0)\n> +\tUniqueFD memfd = MemFd::create(name.c_str(), size);\n> +\tif (!memfd.isValid())\n> \t\treturn;\n> \n> -\tfd_ = SharedFD(std::move(fd));\n> -\tif (!fd_.isValid())\n> -\t\treturn;\n> -\n> -\tif (ftruncate(fd_.get(), size) < 0) {\n> -\t\tfd_ = SharedFD();\n> -\t\treturn;\n> -\t}\n> +\tfd_ = SharedFD(std::move(memfd));\n> \n> \tvoid *mem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,\n> \t\t\t fd_.get(), 0);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 002CFC323E\n\tfor ;\n\tWed, 31 Jul 2024 07:23:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02FDC63374;\n\tWed, 31 Jul 2024 09:23:23 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D6FD6336B\n\tfor ;\n\tWed, 31 Jul 2024 09:23:21 +0200 (CEST)","from mail-ej1-f70.google.com (mail-ej1-f70.google.com\n\t[209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-76-kOHAvUOgNvmmiZkGlBiLlA-1; Wed, 31 Jul 2024 03:23:18 -0400","by mail-ej1-f70.google.com with SMTP id\n\ta640c23a62f3a-a7d2d414949so420124066b.0\n\tfor ;\n\tWed, 31 Jul 2024 00:23:17 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-a7acada2b1asm738303366b.158.2024.07.31.00.23.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 31 Jul 2024 00:23:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"C37NJxdW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722410600;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=eL2XXx2gFlNDge2O/mfSMKcJd2tX3u9FObvaA6xM8XA=;\n\tb=C37NJxdWbDJfOrS40mg9ZRqlmoUuMqUcyJ4RSjUAUJp3jDA+uHThjQRv1yyuUD1/+4V8sc\n\t3nFagMilxUfvQs9uf4uGfhu0rjDjxQekgHgW+gYRqFFDx3uJX6gPJwN/CfXFhBodHutISD\n\tD0pQc417F1TkeVr1XGcQ+9vIq0Dg6O8=","X-MC-Unique":"kOHAvUOgNvmmiZkGlBiLlA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722410596; x=1723015396;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=eL2XXx2gFlNDge2O/mfSMKcJd2tX3u9FObvaA6xM8XA=;\n\tb=Tny44J2FVmFDDZyrfhaxTM6pC9VS1g/KYj3tOSzII4zhUdybxxCLk16kIhOwClvVLQ\n\tamuhR88/cvDt7M1dTvQrEXhW+n7g9giuDE67qS1oqWWkjGT9/3llWY4iROIXqMxCm5Pi\n\ts/cnQaAqDiQE4jrfugfFHam1LLX4YbosUL944T0QOXYNcLOaZOLO2OjEY+IWYSGn10RB\n\tRKKVfJu0k/hD//1dn5Q6+gZLSWW6pBuCCjGzNNzWD0HWjcKKyqyrpRjA+cvRs7BOBqiI\n\teK2RgVFSXcUGEUvT4+xN/Zd3+z+3v+PGETALlxnvenr1Hv2/aBZ6RIpaeAA2Y9D3c8f4\n\t3Mmg==","X-Gm-Message-State":"AOJu0YweCcQ0hMYtYHaZnUvHGSSWNLQH+WwLs42Bej+EGnlM7WOf+Zjs\n\tVvtZM4zISt7+ZDwyhTY4X0CZHTgi14C8M1j21E6lX7Qtis7D6b8kR0ypuztvnTDY+ITG8WQj3GN\n\t9kzY1YutuarI+NhoLFJY2kKBPNhQoiGGp6q7GYCg2Yr02dqfXMMPUQOG3QttzncivKJiVSNsK6z\n\t2yuNhmcd5PCS80mNiriTSEt+K8ZTU+YiHlwifEND3EfEd2J/M5kX8EJUI=","X-Received":["by 2002:a17:907:7f24:b0:a7a:a06b:eec8 with SMTP id\n\ta640c23a62f3a-a7d400ad8e6mr820884266b.37.1722410596426; \n\tWed, 31 Jul 2024 00:23:16 -0700 (PDT)","by 2002:a17:907:7f24:b0:a7a:a06b:eec8 with SMTP id\n\ta640c23a62f3a-a7d400ad8e6mr820882866b.37.1722410595763; \n\tWed, 31 Jul 2024 00:23:15 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IE6N5HtvU1aLjpUV3xYP27nLhhG5iKO/jv3TrEcH+oM1UoeLOZ9zPC5rfXQkJMa//CGXg/N4Q==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/3] libcamera: base: Add MemFd helper class","In-Reply-To":"<20240730232708.17399-2-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Wed, 31 Jul 2024 02:27:06 +0300\")","References":"<20240730232708.17399-1-laurent.pinchart@ideasonboard.com>\n\t<20240730232708.17399-2-laurent.pinchart@ideasonboard.com>","Date":"Wed, 31 Jul 2024 09:23:13 +0200","Message-ID":"<87v80m9g9q.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30511,"web_url":"https://patchwork.libcamera.org/comment/30511/","msgid":"<172241505090.392292.957399424814304798@ping.linuxembedded.co.uk>","date":"2024-07-31T08:37:30","subject":"Re: [PATCH 1/3] libcamera: base: Add MemFd helper class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-07-31 00:27:06)\n> libcamera creates memfds in two locations already, duplicating some\n> code. Move the code to a new MemFd helper class.\n> \n\nThanks, Moving this out to it's own class makes sense to me.\n\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/base/memfd.h | 34 +++++++++\n> include/libcamera/base/meson.build | 1 +\n> src/libcamera/base/memfd.cpp | 112 ++++++++++++++++++++++++++++\n> src/libcamera/base/meson.build | 1 +\n> src/libcamera/dma_buf_allocator.cpp | 46 +-----------\n> src/libcamera/shared_mem_object.cpp | 21 ++----\n> 6 files changed, 157 insertions(+), 58 deletions(-)\n> create mode 100644 include/libcamera/base/memfd.h\n> create mode 100644 src/libcamera/base/memfd.cpp\n> \n> diff --git a/include/libcamera/base/memfd.h b/include/libcamera/base/memfd.h\n> new file mode 100644\n> index 000000000000..b0edd2de5d83\n> --- /dev/null\n> +++ b/include/libcamera/base/memfd.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas on Board Oy\n> + *\n> + * Anonymous file creation\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class MemFd\n> +{\n> +public:\n> + enum class Seal {\n> + None = 0,\n> + Shrink = (1 << 0),\n> + Grow = (1 << 1),\n> + };\n> +\n> + using Seals = Flags;\n> +\n> + static UniqueFD create(const char *name, std::size_t size,\n> + Seals seals = Seal::None);\n> +};\n> +\n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(MemFd::Seal)\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build\n> index bace25d56b13..2a0cee317204 100644\n> --- a/include/libcamera/base/meson.build\n> +++ b/include/libcamera/base/meson.build\n> @@ -21,6 +21,7 @@ libcamera_base_private_headers = files([\n> 'event_notifier.h',\n> 'file.h',\n> 'log.h',\n> + 'memfd.h',\n> 'message.h',\n> 'mutex.h',\n> 'private.h',\n> diff --git a/src/libcamera/base/memfd.cpp b/src/libcamera/base/memfd.cpp\n> new file mode 100644\n> index 000000000000..0d50e2d64638\n> --- /dev/null\n> +++ b/src/libcamera/base/memfd.cpp\n> @@ -0,0 +1,112 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n\nInconsistent with the header.\n\n> + *\n> + * Anonymous file creation\n> + */\n> +\n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +/**\n> + * \\file base/memfd.h\n> + * \\brief Anonymous file creation\n> + */\n> +\n> +/* uClibc doesn't provide the file sealing API. */\n> +#ifndef __DOXYGEN__\n> +#if not HAVE_FILE_SEALS\n> +#define F_ADD_SEALS 1033\n> +#define F_SEAL_SHRINK 0x0002\n> +#define F_SEAL_GROW 0x0004\n> +#endif\n> +#endif\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(File)\n> +\n> +/**\n> + * \\class MemFd\n> + * \\brief Helper class to created anonymous files\n> + */\n> +\n> +/**\n> + * \\enum MemFd::Seal\n> + * \\brief Seals for the MemFd::create() function\n> + * \\var MemFd::Seal::None\n> + * \\brief No seals (used as default value)\n> + * \\var MemFd::Seal::Shrink\n> + * \\brief Prevent the memfd from shrinking\n> + * \\var MemFd::Seal::Grow\n> + * \\brief Prevent the memfd from growing\n> + */\n> +\n> +/**\n> + * \\typedef MemFd::Seals\n> + * \\brief A bitwise combination of MemFd::Seal values\n> + */\n> +\n> +/**\n> + * \\brief Create an anonymous file\n> + * \\param[in] name The file name (displayed in symbolic links in /proc/self/fd/)\n\nCurious: An anonymous file ... with a name ;-)\n\n> + * \\param[in] size The file size\n> + * \\param[in] seals The file seals\n> + * \\return The descriptor of the anonymous file is creation succeeded, or an\n> + *\n\ns/is/if/\n\n> + * This function is a helper that wraps anonymous file (memfd) creation and\n> + * sets the file size and optional seals.\n> + *\n> + * invalid UniqueFD otherwise\n\nI think I saw Milan already commented here - but this doxy needs fixing\n\n> + */\n> +UniqueFD MemFd::create(const char *name, std::size_t size, Seals seals)\n> +{\n> +#if HAVE_MEMFD_CREATE\n> + int ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> +#else\n> + int ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> +#endif\n> + if (ret < 0) {\n> + ret = errno;\n> + LOG(File, Error)\n> + << \"Failed to allocate memfd storage for \" << name\n> + << \": \" << strerror(ret);\n> + return {};\n> + }\n> +\n> + UniqueFD memfd(ret);\n> +\n> + ret = ftruncate(memfd.get(), size);\n> + if (ret < 0) {\n> + ret = errno;\n> + LOG(File, Error)\n> + << \"Failed to set memfd size for \" << name\n> + << \": \" << strerror(ret);\n> + return {};\n> + }\n> +\n> + if (seals) {\n> + int fileSeals = (seals & Seal::Shrink ? F_SEAL_SHRINK : 0)\n> + | (seals & Seal::Grow ? F_SEAL_GROW : 0);\n> +\n> + ret = fcntl(memfd.get(), F_ADD_SEALS, fileSeals);\n> + if (ret < 0) {\n> + ret = errno;\n> + LOG(File, Error)\n> + << \"Failed to seal the memfd for \" << name\n> + << \": \" << strerror(ret);\n> + return {};\n> + }\n> + }\n> +\n> + return memfd;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build\n> index 7a7fd7e4ca87..4a228d335ba4 100644\n> --- a/src/libcamera/base/meson.build\n> +++ b/src/libcamera/base/meson.build\n> @@ -10,6 +10,7 @@ libcamera_base_sources = files([\n> 'file.cpp',\n> 'flags.cpp',\n> 'log.cpp',\n> + 'memfd.cpp',\n> 'message.cpp',\n> 'mutex.cpp',\n> 'object.cpp',\n> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp\n> index c06eca7d04ec..be6efb89fbb7 100644\n> --- a/src/libcamera/dma_buf_allocator.cpp\n> +++ b/src/libcamera/dma_buf_allocator.cpp\n> @@ -13,7 +13,6 @@\n> #include \n> #include \n> #include \n> -#include \n> #include \n> #include \n> \n> @@ -22,6 +21,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> /**\n> * \\file dma_buf_allocator.cpp\n> @@ -126,54 +126,16 @@ DmaBufAllocator::~DmaBufAllocator() = default;\n> * \\brief Check if the DmaBufAllocator instance is valid\n> * \\return True if the DmaBufAllocator is valid, false otherwise\n> */\n> -\n> -/* uClibc doesn't provide the file sealing API. */\n> -#ifndef __DOXYGEN__\n> -#if not HAVE_FILE_SEALS\n> -#define F_ADD_SEALS 1033\n> -#define F_SEAL_SHRINK 0x0002\n> -#endif\n> -#endif\n> -\n> UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)\n> {\n> /* Size must be a multiple of the page size. Round it up. */\n> std::size_t pageMask = sysconf(_SC_PAGESIZE) - 1;\n> size = (size + pageMask) & ~pageMask;\n> \n> -#if HAVE_MEMFD_CREATE\n> - int ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> -#else\n> - int ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC);\n> -#endif\n> - if (ret < 0) {\n> - ret = errno;\n> - LOG(DmaBufAllocator, Error)\n> - << \"Failed to allocate memfd storage for \" << name\n> - << \": \" << strerror(ret);\n> - return {};\n> - }\n> -\n> - UniqueFD memfd(ret);\n> -\n> - ret = ftruncate(memfd.get(), size);\n> - if (ret < 0) {\n> - ret = errno;\n> - LOG(DmaBufAllocator, Error)\n> - << \"Failed to set memfd size for \" << name\n> - << \": \" << strerror(ret);\n> - return {};\n> - }\n> -\n> /* udmabuf dma-buffers *must* have the F_SEAL_SHRINK seal. */\n> - ret = fcntl(memfd.get(), F_ADD_SEALS, F_SEAL_SHRINK);\n> - if (ret < 0) {\n> - ret = errno;\n> - LOG(DmaBufAllocator, Error)\n> - << \"Failed to seal the memfd for \" << name\n> - << \": \" << strerror(ret);\n> + UniqueFD memfd = MemFd::create(name, size, MemFd::Seal::Shrink);\n> + if (!memfd.isValid())\n\nWell that all tidies up nicely.\n\n\nWith the small fixes\n\n\nReviewed-by: Kieran Bingham \n\n> return {};\n> - }\n> \n> struct udmabuf_create create;\n> \n> @@ -182,7 +144,7 @@ UniqueFD DmaBufAllocator::allocFromUDmaBuf(const char *name, std::size_t size)\n> create.offset = 0;\n> create.size = size;\n> \n> - ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create);\n> + int ret = ::ioctl(providerHandle_.get(), UDMABUF_CREATE, &create);\n> if (ret < 0) {\n> ret = errno;\n> LOG(DmaBufAllocator, Error)\n> diff --git a/src/libcamera/shared_mem_object.cpp b/src/libcamera/shared_mem_object.cpp\n> index 809fbdaf95de..022645e71a35 100644\n> --- a/src/libcamera/shared_mem_object.cpp\n> +++ b/src/libcamera/shared_mem_object.cpp\n> @@ -13,9 +13,9 @@\n> #include \n> #include \n> #include \n> -#include \n> #include \n> -#include \n> +\n> +#include \n> \n> /**\n> * \\file shared_mem_object.cpp\n> @@ -58,22 +58,11 @@ SharedMem::SharedMem() = default;\n> */\n> SharedMem::SharedMem(const std::string &name, std::size_t size)\n> {\n> -#if HAVE_MEMFD_CREATE\n> - int fd = memfd_create(name.c_str(), MFD_CLOEXEC);\n> -#else\n> - int fd = syscall(SYS_memfd_create, name.c_str(), MFD_CLOEXEC);\n> -#endif\n> - if (fd < 0)\n> + UniqueFD memfd = MemFd::create(name.c_str(), size);\n> + if (!memfd.isValid())\n> return;\n> \n> - fd_ = SharedFD(std::move(fd));\n> - if (!fd_.isValid())\n> - return;\n> -\n> - if (ftruncate(fd_.get(), size) < 0) {\n> - fd_ = SharedFD();\n> - return;\n> - }\n> + fd_ = SharedFD(std::move(memfd));\n> \n> void *mem = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED,\n> fd_.get(), 0);\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6913BBDC71\n\tfor ;\n\tWed, 31 Jul 2024 08:37:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18C5A63374;\n\tWed, 31 Jul 2024 10:37:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCD8E6336B\n\tfor ;\n\tWed, 31 Jul 2024 10:37:33 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D83F97E4;\n\tWed, 31 Jul 2024 10:36:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"L2O2TJE+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722415006;\n\tbh=nUvSmYHNsWLrKMwz1tOdlKJDHszIGDwS+98eo2O+kOI=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=L2O2TJE+7ULkVrB3c/GxKn6wGnMJ7KMv+QnDTDFefd2BUbhfb9FMs8sAPZ5QgjHiJ\n\tbDxV771KzK3l6OZzEtHLZzOgY2FKvuU5KEyE7Wu2dqnbnNn4JpbWilOow/dy5c0pxv\n\tII5eVoMvBMfF2aVxd/j27ho0q52wg+fgPWn1Mzo4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240730232708.17399-2-laurent.pinchart@ideasonboard.com>","References":"<20240730232708.17399-1-laurent.pinchart@ideasonboard.com>\n\t<20240730232708.17399-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [PATCH 1/3] libcamera: base: Add MemFd helper class","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 31 Jul 2024 09:37:30 +0100","Message-ID":"<172241505090.392292.957399424814304798@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]