From patchwork Sun Dec 15 23:01:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22321 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 DE499C32F6 for ; Sun, 15 Dec 2024 23:02:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8689467F33; Mon, 16 Dec 2024 00:02:27 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AQn5DpH8"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2184F67F1D for ; Mon, 16 Dec 2024 00:02:24 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D8E502C6; Mon, 16 Dec 2024 00:01:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303708; bh=tl24tobVxMQFnd3EKwnlIOCXTZiAunsUHKJ9v+NMni8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AQn5DpH8NF8pwcBoEeo4b6gE843BxbQrNOKRvjLBE6F/YFREVT/mKL/SqUk3E+XIw /P+62v27JG0QkZoIxk2rIytmx2GsjXz9fwexG5Uatvv4XryrxefyPCAD5ah3DmchXN rmMcqV2iGoPBjS9JfSjuQYvitdsDqh95TYBYQbJU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 1/8] Documentation: coding-style: Document usage of classes for strings Date: Mon, 16 Dec 2024 01:01:59 +0200 Message-ID: <20241215230206.11002-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" C++ has three main ways to handle strings: std::string, std::string_view and C-style strings. Document their pros and cons and the rules governing their usage in libcamera. Signed-off-by: Laurent Pinchart --- Documentation/coding-style.rst | 98 ++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index 6ac3a4a0d5175118..dc4b093b58a99006 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -269,6 +269,104 @@ the compiler select the right function. This avoids potential errors such as calling abs(int) with a float argument, performing an unwanted implicit integer conversion. For this reason, cmath is preferred over math.h. +Strings +~~~~~~~ + +This section focusses on strings as a sequence of characters represented by the +`char` type, as those are the only strings used in libcamera. The principles +are however equally applicable to other types of strings. + +C++ includes multiple standard ways to represent and handle strings. Each of +them have pros and cons and different usage patterns. + +1. C-style strings + + C-style strings are null-terminated arrays of characters of type `char`. + They are represented by a `char *` pointing to the first character. C string + literals (`"Hello world!"`) produce read-only global null-terminated arrays + of type `const char`. + + Handling strings through `char *` relies on assumptions that are not + enforced at compile time: the memory must not be freed as long as the + pointer remains valid, and the string must be null-terminated. This causes a + risk of use-after-free or buffer out-of-bounds reads. Furthermore, as the + size of the underlying memory is not bundled with the pointer, handling + writable strings as a `char *` risks buffer out-of-bounds writes. + +2. `std::string` class + + The `std::string` class is the main C++ data type to represent strings. The + class holds the string data as a null-terminated array of characters, as + well as the length of the array. `std::string` literals (`"Hello world!"s`) + converts the character array literal into a temporary `std::string` instance + whose lifetime ends with the statement. + + Usage of `std::string` makes string handling safer and convenient thanks to + the integration with the rest of the C++ standard library. This comes at a + cost, as making copies of the class copies the underlying data, requiring + dynamic allocation of memory. This is partially offset by usage of move + constructors or assignment operators. + +3. `std::string_view` class + + This newer addition to the C++ standard library is a non-owning reference to + a characters array. Like the `std::string` class, the `std::string_view` + stores a pointer to the array and a length, but unlike C-style strings and + the `std::string` class, the array is not guaranteed to be null-terminated. + `std::string_view` literals (`"Hello world!"sv`) create a temporary + `std::string_view` instance whose lifetime ends with the statement, but the + underlying character array has global static storage and lifetime. + + String views are useful to represent in a single way strings with + heterogenous underlying storage, as they can be constructed from a `char *` + or a `std::string`. They reduce the risk of buffer out-of-bounds accesses as + they carry the array length, and they speed up handling of substrings as + they don't cause copies, unlike `std::string`. As the string views store a + borrowed reference to the underlying storage, they are prone to + use-after-free issues. The lack of a null terminator furthermore makes them + unsafe to handle strings that need to be passed to C functions that assume + null-terminated strings (such as most of the C standard library functions). + + C++17 lacks many functions that make `std::string_view` usage convenient. + This includes `operator+()` to concatenate a `std::string` and a + `std::string_view`, as well as heterogenous lookup functions for containers + (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2363r5.html). + libcamera works around the former by defining the missing operators, and the + latter by using the `find()` function instead of `at()` or `operator[]()`. + Those issues are properly addressed in C++26. + +With those pros, cons are caveats in mind, libcamera follows a set of rules +that governs selection of appropriate string types for function arguments: + +* If the function is internal to a compilation unit, and all callers use the + same string type for the function argument, use that type (by reference or + value, as appropriate). +* If the function needs to modify the string, make a copy, or convert it to a + `std::string` for any other reason, pass a `std::string` by value. Use + `std::move()` in the caller if the string is not needed after the function + call, as well as inside the function to move the string to a local copy if + needed. This minimizes the number of copies in all use cases. +* If the function needs to pass the string to a C function that takes a + `const char *`, and the caller may reasonably use a C string literal, pass a + `const char *`. This avoids copies when the caller uses a `char *` pointer or + a C string literal, and avoids buffer out-of-bound reads that could occur with + a non null-terminated `std::string_view`. +* If the function only needs to pass the string to other functions that take a + `const std::string &` reference, pass a `const std::string &`. This minimizes + the construction of `std::string` instances when the caller already holds an + instance. +* If the function only reads from the string, doesn't rely on it being + null-terminated, and can reasonably be called with either C strings, C string + literals, or `std::string` instances, pass a `std::string_view` instance. This + improves safety of the code without impacting performance. +* Do not use `std::string_view` in public API function parameters. Previous + rules showed that selection of the optimal string type for function + parameters depends on the internal implementation of the function, which + could change over time. This conflicts with the stability requirements of the + libcamera public API. Furthermore, as C++17 makes `std::string_view` usage + inconvenient in many cases, we don't want to force that inconvenience upon + libcamera users. + Documentation ------------- From patchwork Sun Dec 15 23:02:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22322 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 12977C32F6 for ; Sun, 15 Dec 2024 23:02:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BB31067F32; Mon, 16 Dec 2024 00:02:28 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="LCpaqpXv"; 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 DBFF067F1D for ; Mon, 16 Dec 2024 00:02:25 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 499B92C6; Mon, 16 Dec 2024 00:01:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303709; bh=uauMBSHP1hmbgl27o/OtM059YBY7Ft15XGKnQd3nRuw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LCpaqpXvmJInQ8Jdbpa/AKzyrJ1nbjwoiZyMOugtBeID6UCAdfkJgF0qb+UnB6Ray L/2ZSeKBvysgDam19XtC/5yzPlhtIbMFwtQnf+w3oVsp2e1EP/ddQqLYiOEI2HHd8M aa+TldSlwc4Kc7/aavkvLumNqDhS7buXYLcS7lSA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 2/8] libcamera: base: utils: Add string_view concatenation operators Date: Mon, 16 Dec 2024 01:02:00 +0200 Message-ID: <20241215230206.11002-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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 operator+() overloads to concatenate std::string and std::string_view instances have been introduced in C++26 only. Add a local implementation in utils.cpp, conditioned by a meson check for availability in the standard C++ library. Signed-off-by: Laurent Pinchart --- include/libcamera/base/utils.h | 47 ++++++++++++++++++++++++++++++++++ src/libcamera/base/meson.build | 12 +++++++++ test/utils.cpp | 34 ++++++++++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h index 780aeda6a0ce732a..dd012fd58501cd8d 100644 --- a/include/libcamera/base/utils.h +++ b/include/libcamera/base/utils.h @@ -433,6 +433,53 @@ private: template std::basic_ostream &operator<<(std::basic_ostream &os, const utils::Duration &d); + +#if not HAVE_STD_STRING_VIEW_OPERATOR_PLUS +template +std::basic_string +operator+(const std::basic_string &lhs, + std::basic_string_view rhs) +{ + std::basic_string str; + + str.reserve(lhs.size() + rhs.size()); + str.append(lhs); + str.append(rhs); + + return str; +} + +template +std::basic_string +operator+(std::basic_string_view lhs, + const std::basic_string &rhs) +{ + std::basic_string str; + + str.reserve(lhs.size() + rhs.size()); + str.append(lhs); + str.append(rhs); + + return str; +} + +template +std::basic_string +operator+(std::basic_string &&lhs, + std::basic_string_view rhs) +{ + return std::move(lhs.append(rhs)); +} + +template +std::basic_string +operator+(std::basic_string_view lhs, + std::basic_string &&rhs) +{ + return std::move(rhs.insert(0, lhs)); +} +#endif /* HAVE_STD_STRING_VIEW_OPERATOR_PLUS */ + #endif } /* namespace libcamera */ diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build index a742dfdfeb24f9a9..1643d6034921a0cb 100644 --- a/src/libcamera/base/meson.build +++ b/src/libcamera/base/meson.build @@ -41,6 +41,18 @@ if libunwind.found() config_h.set('HAVE_UNWIND', 1) endif +if cxx.compiles(''' +#include +#include + +int main() +{ + std::string s = std::string() + std::string_view(); + return s.size(); +}''', name : 'concatenation of std::string and std::string_view') + config_h.set('HAVE_STD_STRING_VIEW_OPERATOR_PLUS', 1) +endif + libcamera_base_deps = [ libatomic, libdw, diff --git a/test/utils.cpp b/test/utils.cpp index d25475cb93b96a0a..29e3c26406b4f72e 100644 --- a/test/utils.cpp +++ b/test/utils.cpp @@ -170,6 +170,36 @@ protected: return TestPass; } + int testStringView() + { + std::string s{ "Hello" }; + std::string_view sv{ "World!" }; + + if (s + sv != "HelloWorld!") { + cerr << "operator+(const std::string &, std::string_view) test failed"; + return TestFail; + } + + if (sv + s != "World!Hello") { + cerr << "operator+(std::string_view, const std::string &) test failed"; + return TestFail; + } + + if (std::move(s) + sv != "HelloWorld!") { + cerr << "operator+(std::string &&, std::string_view) test failed"; + return TestFail; + } + + s = "Hello"; + + if (sv + std::move(s) != "World!Hello") { + cerr << "operator+(std::string_view, std::string &&) test failed"; + return TestFail; + } + + return TestPass; + } + int run() { /* utils::hex() test. */ @@ -307,6 +337,10 @@ protected: if (testDuration() != TestPass) return TestFail; + /* std::string_view operator+() test. */ + if (testStringView() != TestPass) + return TestFail; + return TestPass; } }; From patchwork Sun Dec 15 23:02:01 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22323 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 B6EA7C32F6 for ; Sun, 15 Dec 2024 23:02:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AF06E67F37; Mon, 16 Dec 2024 00:02:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="VkqNKq4o"; 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 C4B8567F27 for ; Mon, 16 Dec 2024 00:02:26 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 96E1A2C6; Mon, 16 Dec 2024 00:01:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303710; bh=I6tt0xIyGyxaF9ry/0EGE4JJlwugKl+FhWvr+mPUC2k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VkqNKq4o6k/hGNH1sqt2z5a7h/VGFIL0x25lJZgK0yzcOOJ6pJWL75bxhmUnXl3Ku zSSrv2F6IIJaDyArQdma5gUK+aiWXb8843om1QqfsdeFTmGiRi5yaTzNYYHT0Vvyzw bHJMUdDGz3CD5v3Hxzb1ClX8J3cg/+VJ3b6GXXSs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 3/8] libcamera: base: file: Fix string usage in class API Date: Mon, 16 Dec 2024 01:02:01 +0200 Message-ID: <20241215230206.11002-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" Follow the string usage guidelines documented in the libcamera coding style in the File class API. The rationale is explained in the guidelines. Signed-off-by: Laurent Pinchart --- include/libcamera/base/file.h | 6 +++--- src/libcamera/base/file.cpp | 14 +++++++------- src/libcamera/sysfs.cpp | 2 +- test/camera/camera_reconfigure.cpp | 2 +- test/hotplug-cameras.cpp | 3 ++- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/include/libcamera/base/file.h b/include/libcamera/base/file.h index 6d3f106d5900f7a7..dc2841d92c8f3573 100644 --- a/include/libcamera/base/file.h +++ b/include/libcamera/base/file.h @@ -40,12 +40,12 @@ public: using OpenMode = Flags; - File(const std::string &name); + File(std::string name); File(); ~File(); const std::string &fileName() const { return name_; } - void setFileName(const std::string &name); + void setFileName(std::string name); bool exists() const; bool open(OpenMode mode); @@ -66,7 +66,7 @@ public: MapFlags flags = MapFlag::NoOption); bool unmap(uint8_t *addr); - static bool exists(const std::string &name); + static bool exists(const char *name); private: LIBCAMERA_DISABLE_COPY(File) diff --git a/src/libcamera/base/file.cpp b/src/libcamera/base/file.cpp index 2b83a51775b04661..5cff82064101bc2b 100644 --- a/src/libcamera/base/file.cpp +++ b/src/libcamera/base/file.cpp @@ -83,8 +83,8 @@ LOG_DEFINE_CATEGORY(File) * Upon construction the File object is closed and shall be opened with open() * before performing I/O operations. */ -File::File(const std::string &name) - : name_(name), mode_(OpenModeFlag::NotOpen), error_(0) +File::File(std::string name) + : name_(std::move(name)), mode_(OpenModeFlag::NotOpen), error_(0) { } @@ -126,7 +126,7 @@ File::~File() * * Any memory mapping associated with the File is unmapped. */ -void File::setFileName(const std::string &name) +void File::setFileName(std::string name) { if (isOpen()) { LOG(File, Error) @@ -136,7 +136,7 @@ void File::setFileName(const std::string &name) unmapAll(); - name_ = name; + name_ = std::move(name); } /** @@ -151,7 +151,7 @@ void File::setFileName(const std::string &name) */ bool File::exists() const { - return exists(name_); + return exists(name_.c_str()); } /** @@ -464,10 +464,10 @@ void File::unmapAll() * \param[in] name The file name * \return True if the file exists, false otherwise */ -bool File::exists(const std::string &name) +bool File::exists(const char *name) { struct stat st; - int ret = stat(name.c_str(), &st); + int ret = stat(name, &st); if (ret < 0) return false; diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp index 3d9885b080c6f911..cbde72d86fd62b41 100644 --- a/src/libcamera/sysfs.cpp +++ b/src/libcamera/sysfs.cpp @@ -92,7 +92,7 @@ std::string firmwareNodePath(const std::string &device) /* Lookup for ACPI-based systems */ node = device + "/firmware_node/path"; - if (File::exists(node)) { + if (File::exists(node.c_str())) { std::ifstream file(node); if (!file.is_open()) return {}; diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp index 06c87730a1305952..51bd1bf3493d9a86 100644 --- a/test/camera/camera_reconfigure.cpp +++ b/test/camera/camera_reconfigure.cpp @@ -179,7 +179,7 @@ private: continue; string pname("/proc/" + string(ptr->d_name) + "/comm"); - if (File::exists(pname)) { + if (File::exists(pname.c_str())) { ifstream pfile(pname.c_str()); string comm; getline(pfile, comm); diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp index 530e9a31120970de..37652c9941e75bb5 100644 --- a/test/hotplug-cameras.cpp +++ b/test/hotplug-cameras.cpp @@ -73,7 +73,8 @@ protected: dir = opendir(uvcDriverDir_.c_str()); /* Find a UVC device directory, which we can bind/unbind. */ while ((dirent = readdir(dir)) != nullptr) { - if (!File::exists(uvcDriverDir_ + dirent->d_name + "/video4linux")) + std::string fileName = uvcDriverDir_ + dirent->d_name + "/video4linux"; + if (!File::exists(fileName.c_str())) continue; uvcDeviceDir = dirent->d_name; From patchwork Sun Dec 15 23:02:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22324 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 29C42C32F6 for ; Sun, 15 Dec 2024 23:02:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9B38C67F3F; Mon, 16 Dec 2024 00:02:36 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="uus22VyD"; 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 509D267F35 for ; Mon, 16 Dec 2024 00:02:28 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F0514675; Mon, 16 Dec 2024 00:01:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303712; bh=H3eOHpCnviI3I3rNvPcS0PHYuFpNPh0DjayX9XAj4ms=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uus22VyD5WhCikXQ6j41GHVav7SaQPces2j10Zfdx5MQlMd7LGBikstZl5lTRgxPr Rl+8TyvA7FANafU1zo2YA1x7fR34qOgZYq6G7Gau63DbwjIswu2p+NN7wJKNiBr6IB tocDTdVxMzUh30GaN6QPySTLuT53ePsJO/9QH1II= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 4/8] libcamera: base: log: Use std::string_view Date: Mon, 16 Dec 2024 01:02:02 +0200 Message-ID: <20241215230206.11002-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" Replace usage of const std::string references passed to class member functions with std::string_view. This allows using static C string literals in the callers without the overhead of constructing a std::string instance. As string views are not guaranteed to be zero-terminated, two usages need to be modified: - The call to syslog() is changed to use "%.*s" instead of "%s" - The strtoul() call is replaced with std::from_chars() Signed-off-by: Laurent Pinchart --- include/libcamera/base/log.h | 3 ++- src/libcamera/base/log.cpp | 31 ++++++++++++++++--------------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h index 620930125f2c1fa2..c22bafbd5b8019a2 100644 --- a/include/libcamera/base/log.h +++ b/include/libcamera/base/log.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include @@ -60,7 +61,7 @@ class LogMessage public: LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, - const std::string &prefix = std::string()); + std::string_view prefix = {}); LogMessage(LogMessage &&); ~LogMessage(); diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp index 3a656b8f099f673f..a2a59fbf26195340 100644 --- a/src/libcamera/base/log.cpp +++ b/src/libcamera/base/log.cpp @@ -8,11 +8,11 @@ #include #include +#include #include #include #include #include -#include #include #include #include @@ -112,11 +112,11 @@ public: bool isValid() const; void write(const LogMessage &msg); - void write(const std::string &msg); + void write(std::string_view msg); private: - void writeSyslog(LogSeverity severity, const std::string &msg); - void writeStream(const std::string &msg); + void writeSyslog(LogSeverity severity, std::string_view msg); + void writeStream(std::string_view msg); std::ostream *stream_; LoggingTarget target_; @@ -260,7 +260,7 @@ void LogOutput::write(const LogMessage &msg) * \brief Write string to log output * \param[in] str String to write */ -void LogOutput::write(const std::string &str) +void LogOutput::write(std::string_view str) { switch (target_) { case LoggingTargetSyslog: @@ -275,14 +275,15 @@ void LogOutput::write(const std::string &str) } } -void LogOutput::writeSyslog(LogSeverity severity, const std::string &str) +void LogOutput::writeSyslog(LogSeverity severity, std::string_view str) { - syslog(log_severity_to_syslog(severity), "%s", str.c_str()); + syslog(log_severity_to_syslog(severity), "%.*s", + static_cast(str.size()), str.data()); } -void LogOutput::writeStream(const std::string &str) +void LogOutput::writeStream(std::string_view str) { - stream_->write(str.c_str(), str.size()); + stream_->write(str.data(), str.size()); stream_->flush(); } @@ -311,7 +312,7 @@ private: void parseLogFile(); void parseLogLevels(); - static LogSeverity parseLogLevel(const std::string &level); + static LogSeverity parseLogLevel(std::string_view level); friend LogCategory; void registerCategory(LogCategory *category); @@ -675,7 +676,7 @@ void Logger::parseLogLevels() * * \return The log severity, or LogInvalid if the string is invalid */ -LogSeverity Logger::parseLogLevel(const std::string &level) +LogSeverity Logger::parseLogLevel(std::string_view level) { static const char *const names[] = { "DEBUG", @@ -688,9 +689,9 @@ LogSeverity Logger::parseLogLevel(const std::string &level) int severity; if (std::isdigit(level[0])) { - char *endptr; - severity = strtoul(level.c_str(), &endptr, 10); - if (*endptr != '\0' || severity > LogFatal) + std::from_chars_result res; + res = std::from_chars(level.begin(), level.end(), severity); + if (res.ptr != level.end() || severity > LogFatal) severity = LogInvalid; } else { severity = LogInvalid; @@ -874,7 +875,7 @@ const LogCategory &LogCategory::defaultCategory() */ LogMessage::LogMessage(const char *fileName, unsigned int line, const LogCategory &category, LogSeverity severity, - const std::string &prefix) + std::string_view prefix) : category_(category), severity_(severity), prefix_(prefix) { init(fileName, line); From patchwork Sun Dec 15 23:02:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22325 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 753B1C32F6 for ; Sun, 15 Dec 2024 23:02:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E621D67F45; Mon, 16 Dec 2024 00:02:37 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OY6LTa9b"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 84F9967F1D for ; Mon, 16 Dec 2024 00:02:29 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5025B2C6; Mon, 16 Dec 2024 00:01:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303713; bh=jicbLubGAelOBp5iooZvoFoBWq8tpZBmR9fVz8vVW0Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OY6LTa9bGZ+zBaw3oEZNEXATBsFb+pXH1n+RW8EwCDP+mztEp1m4+59EXI6iz4Paa pgxVOefYaQ6DgFVQTYvUbYf25NPz1CRQbwWOvQeOEiJQcGu095CI0VZ2i10KNJ4T3/ CKbu0kXW+ch0hnU3lKDxLySiBkR1pvS0GH3MAM4s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 5/8] libcamera: base: utils: Use std::string_view Date: Mon, 16 Dec 2024 01:02:03 +0200 Message-ID: <20241215230206.11002-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" Replace usage of const std::string references passed to class member functions with std::string_view. This allows using static C string literals in the callers without the overhead of constructing a std::string instance. As std::string can't be implicitly constructed from std::string_view, an explicit construction is added at the end of dirname(). Signed-off-by: Laurent Pinchart --- include/libcamera/base/utils.h | 15 ++++++++------- src/libcamera/base/utils.cpp | 14 +++++++------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h index dd012fd58501cd8d..3ef1c0e87383f8d2 100644 --- a/include/libcamera/base/utils.h +++ b/include/libcamera/base/utils.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -38,7 +39,7 @@ namespace utils { const char *basename(const char *path); char *secure_getenv(const char *name); -std::string dirname(const std::string &path); +std::string dirname(std::string_view path); template std::vector map_keys(const T &map) @@ -143,7 +144,7 @@ size_t strlcpy(char *dst, const char *src, size_t size); #ifndef __DOXYGEN__ template -std::string join(const Container &items, const std::string &sep, UnaryOp op) +std::string join(const Container &items, std::string_view sep, UnaryOp op) { std::ostringstream ss; bool first = true; @@ -162,7 +163,7 @@ std::string join(const Container &items, const std::string &sep, UnaryOp op) } template -std::string join(const Container &items, const std::string &sep) +std::string join(const Container &items, std::string_view sep) { std::ostringstream ss; bool first = true; @@ -181,7 +182,7 @@ std::string join(const Container &items, const std::string &sep) } #else template -std::string join(const Container &items, const std::string &sep, UnaryOp op = nullptr); +std::string join(const Container &items, std::string_view sep, UnaryOp op = nullptr); #endif namespace details { @@ -189,7 +190,7 @@ namespace details { class StringSplitter { public: - StringSplitter(const std::string &str, const std::string &delim); + StringSplitter(std::string_view str, std::string_view delim); class iterator { @@ -238,9 +239,9 @@ private: } /* namespace details */ -details::StringSplitter split(const std::string &str, const std::string &delim); +details::StringSplitter split(std::string_view str, std::string_view delim); -std::string toAscii(const std::string &str); +std::string toAscii(std::string_view str); std::string libcameraBuildPath(); std::string libcameraSourcePath(); diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp index bcfc1941a92a3d69..886aa9ecdc2bddfe 100644 --- a/src/libcamera/base/utils.cpp +++ b/src/libcamera/base/utils.cpp @@ -80,7 +80,7 @@ char *secure_getenv(const char *name) * * \return A string of the directory component of the path */ -std::string dirname(const std::string &path) +std::string dirname(std::string_view path) { if (path.empty()) return "."; @@ -116,7 +116,7 @@ std::string dirname(const std::string &path) pos--; } - return path.substr(0, pos + 1); + return std::string(path.substr(0, pos + 1)); } /** @@ -247,7 +247,7 @@ size_t strlcpy(char *dst, const char *src, size_t size) return strlen(src); } -details::StringSplitter::StringSplitter(const std::string &str, const std::string &delim) +details::StringSplitter::StringSplitter(std::string_view str, std::string_view delim) : str_(str), delim_(delim) { } @@ -278,7 +278,7 @@ std::string details::StringSplitter::iterator::operator*() const /** * \fn template \ - * std::string utils::join(const Container &items, const std::string &sep, UnaryOp op) + * std::string utils::join(const Container &items, std::string_view sep, UnaryOp op) * \brief Join elements of a container in a string with a separator * \param[in] items The container * \param[in] sep The separator to add between elements @@ -293,7 +293,7 @@ std::string details::StringSplitter::iterator::operator*() const */ /** - * \fn split(const std::string &str, const std::string &delim) + * \fn split(std::string_view str, std::string_view delim) * \brief Split a string based on a delimiter * \param[in] str The string to split * \param[in] delim The delimiter string @@ -305,7 +305,7 @@ std::string details::StringSplitter::iterator::operator*() const * \return An object that can be used in a range-based for loop to iterate over * the substrings */ -details::StringSplitter split(const std::string &str, const std::string &delim) +details::StringSplitter split(std::string_view str, std::string_view delim) { /** \todo Try to avoid copies of str and delim */ return details::StringSplitter(str, delim); @@ -319,7 +319,7 @@ details::StringSplitter split(const std::string &str, const std::string &delim) * * \return A string equal to \a str stripped out of all non-ASCII characters */ -std::string toAscii(const std::string &str) +std::string toAscii(std::string_view str) { std::string ret; for (const char &c : str) From patchwork Sun Dec 15 23:02:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22326 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 D339BC32F6 for ; Sun, 15 Dec 2024 23:02:40 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3960867F42; Mon, 16 Dec 2024 00:02:40 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="cxGMXoSw"; 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 0E6D367F3C for ; Mon, 16 Dec 2024 00:02:31 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A91E22C6; Mon, 16 Dec 2024 00:01:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303715; bh=tCzOv7ynYDErg4/AoDhNXCoBUptU08pA90XBObCwPJ4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=cxGMXoSwHdIym1o0+5+uy2VQOPitPvT8S3Qf4mvdBRixgMEXGUuJRUfo+U5I3ajeO v1m+HjCn/ec2IByvxR+Zi6wPcM2Encrkc6pX+eD6zBNV4rPGAZo6YPv2CwG+sgfz8J E8MZ864d5FaImFKycSLYy3kelA8KiTxZJ76lLdzo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 6/8] libcamera: Update string usage in internal APIs Date: Mon, 16 Dec 2024 01:02:04 +0200 Message-ID: <20241215230206.11002-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" Follow the string usage guidelines documented in the libcamera coding style in most of the internal APIs. This showcases usage of std::string_view where applicable. The rationale is explained in the guidelines. Signed-off-by: Laurent Pinchart --- .../internal/camera_sensor_properties.h | 4 ++-- include/libcamera/internal/converter.h | 3 ++- .../internal/converter/converter_v4l2_m2m.h | 3 +-- include/libcamera/internal/device_enumerator.h | 8 ++++---- include/libcamera/internal/formats.h | 3 ++- include/libcamera/internal/ipa_module.h | 3 ++- include/libcamera/internal/ipa_proxy.h | 7 ++++--- include/libcamera/internal/media_device.h | 9 +++++---- include/libcamera/internal/pipeline_handler.h | 7 ++++--- .../internal/software_isp/software_isp.h | 4 ++-- include/libcamera/internal/sysfs.h | 3 ++- include/libcamera/internal/v4l2_device.h | 3 ++- include/libcamera/internal/v4l2_subdevice.h | 3 ++- include/libcamera/internal/v4l2_videodevice.h | 7 ++++--- src/libcamera/device_enumerator.cpp | 10 +++++----- src/libcamera/formats.cpp | 2 +- src/libcamera/ipa_module.cpp | 2 +- src/libcamera/ipa_proxy.cpp | 8 ++++---- src/libcamera/media_device.cpp | 16 ++++++++-------- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 ++- src/libcamera/pipeline_handler.cpp | 6 +++--- .../sensor/camera_sensor_properties.cpp | 4 ++-- src/libcamera/software_isp/software_isp.cpp | 3 ++- src/libcamera/sysfs.cpp | 8 +++++--- src/libcamera/v4l2_device.cpp | 2 +- src/libcamera/v4l2_subdevice.cpp | 3 +-- src/libcamera/v4l2_videodevice.cpp | 7 +++---- 27 files changed, 76 insertions(+), 65 deletions(-) diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h index d7d4dab626df7b82..b44093906d6390c0 100644 --- a/include/libcamera/internal/camera_sensor_properties.h +++ b/include/libcamera/internal/camera_sensor_properties.h @@ -9,7 +9,7 @@ #include #include -#include +#include #include #include @@ -24,7 +24,7 @@ struct CameraSensorProperties { uint8_t hblankDelay; }; - static const CameraSensorProperties *get(const std::string &sensor); + static const CameraSensorProperties *get(std::string_view sensor); Size unitCellSize; std::map testPatternModes; diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h index ffbb6f345cd5ed4f..fb8d81898e90bcb8 100644 --- a/include/libcamera/internal/converter.h +++ b/include/libcamera/internal/converter.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -44,7 +45,7 @@ public: Converter(MediaDevice *media, Features features = Feature::None); virtual ~Converter(); - virtual int loadConfiguration(const std::string &filename) = 0; + virtual int loadConfiguration(std::string_view filename) = 0; virtual bool isValid() const = 0; diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h index 0bc0d053e2c47d3a..5475cc6b3ac58b30 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include @@ -38,7 +37,7 @@ class V4L2M2MConverter : public Converter public: V4L2M2MConverter(MediaDevice *media); - int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } + int loadConfiguration([[maybe_unused]] std::string_view filename) { return 0; } bool isValid() const { return m2m_ != nullptr; } std::vector formats(PixelFormat input); diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h index db3532a9887af913..c3a09f03433729c4 100644 --- a/include/libcamera/internal/device_enumerator.h +++ b/include/libcamera/internal/device_enumerator.h @@ -20,9 +20,9 @@ class MediaDevice; class DeviceMatch { public: - DeviceMatch(const std::string &driver); + DeviceMatch(std::string_view driver); - void add(const std::string &entity); + void add(std::string_view entity); bool match(const MediaDevice *device) const; @@ -46,9 +46,9 @@ public: Signal<> devicesAdded; protected: - std::unique_ptr createDevice(const std::string &deviceNode); + std::unique_ptr createDevice(std::string_view deviceNode); void addDevice(std::unique_ptr media); - void removeDevice(const std::string &deviceNode); + void removeDevice(std::string_view deviceNode); private: std::vector> devices_; diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h index 6a3e9c16a873e969..bd7ac6ed128f594b 100644 --- a/include/libcamera/internal/formats.h +++ b/include/libcamera/internal/formats.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include @@ -35,7 +36,7 @@ public: static const PixelFormatInfo &info(const PixelFormat &format); static const PixelFormatInfo &info(const V4L2PixelFormat &format); - static const PixelFormatInfo &info(const std::string &name); + static const PixelFormatInfo &info(std::string_view name); unsigned int stride(unsigned int width, unsigned int plane, unsigned int align = 1) const; diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h index 7c49d3f342e5055c..5f7c7e6e9f6987a0 100644 --- a/include/libcamera/internal/ipa_module.h +++ b/include/libcamera/internal/ipa_module.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -23,7 +24,7 @@ namespace libcamera { class IPAModule : public Loggable { public: - explicit IPAModule(const std::string &libPath); + explicit IPAModule(std::string_view libPath); ~IPAModule(); bool isValid() const; diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h index 983bcc5faa4bab3b..93b3b991f8d4b80e 100644 --- a/include/libcamera/internal/ipa_proxy.h +++ b/include/libcamera/internal/ipa_proxy.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include @@ -29,11 +30,11 @@ public: bool isValid() const { return valid_; } - std::string configurationFile(const std::string &name, - const std::string &fallbackName = std::string()) const; + std::string configurationFile(std::string_view name, + std::string_view fallbackName = {}) const; protected: - std::string resolvePath(const std::string &file) const; + std::string resolvePath(std::string_view file) const; bool valid_; ProxyState state_; diff --git a/include/libcamera/internal/media_device.h b/include/libcamera/internal/media_device.h index e412d3a0b7e38cd3..1882b9aeb87f533a 100644 --- a/include/libcamera/internal/media_device.h +++ b/include/libcamera/internal/media_device.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -24,7 +25,7 @@ namespace libcamera { class MediaDevice : protected Loggable { public: - MediaDevice(const std::string &deviceNode); + MediaDevice(std::string_view deviceNode); ~MediaDevice(); bool acquire(); @@ -44,10 +45,10 @@ public: unsigned int hwRevision() const { return hwRevision_; } const std::vector &entities() const { return entities_; } - MediaEntity *getEntityByName(const std::string &name) const; + MediaEntity *getEntityByName(std::string_view name) const; - MediaLink *link(const std::string &sourceName, unsigned int sourceIdx, - const std::string &sinkName, unsigned int sinkIdx); + MediaLink *link(std::string_view sourceName, unsigned int sourceIdx, + std::string_view sinkName, unsigned int sinkIdx); MediaLink *link(const MediaEntity *source, unsigned int sourceIdx, const MediaEntity *sink, unsigned int sinkIdx); MediaLink *link(const MediaPad *source, const MediaPad *sink); diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index fb28a18d0f4668ab..f91d1c6cba842f49 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -62,8 +63,8 @@ public: void completeRequest(Request *request); void cancelRequest(Request *request); - std::string configurationFile(const std::string &subdir, - const std::string &name) const; + std::string configurationFile(std::string_view subdir, + std::string_view name) const; const char *name() const { return name_; } @@ -112,7 +113,7 @@ public: const std::string &name() const { return name_; } static std::vector &factories(); - static const PipelineHandlerFactoryBase *getFactoryByName(const std::string &name); + static const PipelineHandlerFactoryBase *getFactoryByName(std::string_view name); private: static void registerType(PipelineHandlerFactoryBase *factory); diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h index d51b03fd6cd71fce..bc4eb36260e93fbc 100644 --- a/include/libcamera/internal/software_isp/software_isp.h +++ b/include/libcamera/internal/software_isp/software_isp.h @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include @@ -50,7 +50,7 @@ public: ControlInfoMap *ipaControls); ~SoftwareIsp(); - int loadConfiguration([[maybe_unused]] const std::string &filename) { return 0; } + int loadConfiguration([[maybe_unused]] std::string_view filename) { return 0; } bool isValid() const; diff --git a/include/libcamera/internal/sysfs.h b/include/libcamera/internal/sysfs.h index aca60fb6c8ab9a2c..db78d790f3ff23bd 100644 --- a/include/libcamera/internal/sysfs.h +++ b/include/libcamera/internal/sysfs.h @@ -8,6 +8,7 @@ #pragma once #include +#include namespace libcamera { @@ -15,7 +16,7 @@ namespace sysfs { std::string charDevPath(const std::string &deviceNode); -std::string firmwareNodePath(const std::string &device); +std::string firmwareNodePath(std::string_view device); } /* namespace sysfs */ diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index f5aa502482017325..623403dc615ea604 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -50,7 +51,7 @@ public: void updateControlInfo(); protected: - V4L2Device(const std::string &deviceNode); + V4L2Device(std::string_view deviceNode); ~V4L2Device(); int open(unsigned int flags); diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h index 194382f84d97fbbc..d17365e14a231df3 100644 --- a/include/libcamera/internal/v4l2_subdevice.h +++ b/include/libcamera/internal/v4l2_subdevice.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -161,7 +162,7 @@ public: const V4L2SubdeviceCapability &caps() const { return caps_; } static std::unique_ptr - fromEntityName(const MediaDevice *media, const std::string &entity); + fromEntityName(const MediaDevice *media, std::string_view entity); protected: std::string logPrefix() const override; diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index f021c2a0177b4512..45443ed716d7f6f2 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -189,7 +190,7 @@ class V4L2VideoDevice : public V4L2Device public: using Formats = std::map>; - explicit V4L2VideoDevice(const std::string &deviceNode); + explicit V4L2VideoDevice(std::string_view deviceNode); explicit V4L2VideoDevice(const MediaEntity *entity); ~V4L2VideoDevice(); @@ -228,7 +229,7 @@ public: Signal<> dequeueTimeout; static std::unique_ptr - fromEntityName(const MediaDevice *media, const std::string &entity); + fromEntityName(const MediaDevice *media, std::string_view entity); V4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const; @@ -295,7 +296,7 @@ private: class V4L2M2MDevice { public: - V4L2M2MDevice(const std::string &deviceNode); + V4L2M2MDevice(std::string_view deviceNode); ~V4L2M2MDevice(); int open(); diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp index ae17862f676310ef..1efc5b33639f6af8 100644 --- a/src/libcamera/device_enumerator.cpp +++ b/src/libcamera/device_enumerator.cpp @@ -67,7 +67,7 @@ LOG_DEFINE_CATEGORY(DeviceEnumerator) * \brief Construct a media device search pattern * \param[in] driver The Linux device driver name that created the media device */ -DeviceMatch::DeviceMatch(const std::string &driver) +DeviceMatch::DeviceMatch(std::string_view driver) : driver_(driver) { } @@ -76,9 +76,9 @@ DeviceMatch::DeviceMatch(const std::string &driver) * \brief Add a media entity name to the search pattern * \param[in] entity The name of the entity in the media graph */ -void DeviceMatch::add(const std::string &entity) +void DeviceMatch::add(std::string_view entity) { - entities_.push_back(entity); + entities_.push_back(std::string(entity)); } /** @@ -215,7 +215,7 @@ DeviceEnumerator::~DeviceEnumerator() * * \return Created media device instance on success, or nullptr otherwise */ -std::unique_ptr DeviceEnumerator::createDevice(const std::string &deviceNode) +std::unique_ptr DeviceEnumerator::createDevice(std::string_view deviceNode) { std::unique_ptr media = std::make_unique(deviceNode); @@ -272,7 +272,7 @@ void DeviceEnumerator::addDevice(std::unique_ptr media) * enumerator with addDevice(). The media device's MediaDevice::disconnected * signal is emitted. */ -void DeviceEnumerator::removeDevice(const std::string &deviceNode) +void DeviceEnumerator::removeDevice(std::string_view deviceNode) { std::shared_ptr media; diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp index bfcdfc08960dbbad..112dfe66b4747516 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -1037,7 +1037,7 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format) * \return The PixelFormatInfo describing the PixelFormat matching the * \a name if known, or an invalid PixelFormatInfo otherwise */ -const PixelFormatInfo &PixelFormatInfo::info(const std::string &name) +const PixelFormatInfo &PixelFormatInfo::info(std::string_view name) { for (const auto &info : pixelFormatInfo) { if (info.second.name == name) diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp index 9ca74be6e100a8fa..90e3148d236f3a3c 100644 --- a/src/libcamera/ipa_module.cpp +++ b/src/libcamera/ipa_module.cpp @@ -254,7 +254,7 @@ Span elfLoadSymbol(Span elf, const char *symbol) * The caller shall call the isValid() function after constructing an * IPAModule instance to verify the validity of the IPAModule. */ -IPAModule::IPAModule(const std::string &libPath) +IPAModule::IPAModule(std::string_view libPath) : libPath_(libPath), valid_(false), loaded_(false), dlHandle_(nullptr), ipaCreate_(nullptr) { diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp index 85004737c17100bf..a06bd94ff435021e 100644 --- a/src/libcamera/ipa_proxy.cpp +++ b/src/libcamera/ipa_proxy.cpp @@ -95,8 +95,8 @@ IPAProxy::~IPAProxy() * \return The full path to the IPA configuration file, or an empty string if * no configuration file can be found */ -std::string IPAProxy::configurationFile(const std::string &name, - const std::string &fallbackName) const +std::string IPAProxy::configurationFile(std::string_view name, + std::string_view fallbackName) const { struct stat statbuf; int ret; @@ -178,9 +178,9 @@ std::string IPAProxy::configurationFile(const std::string &name, * \return The full path to the proxy worker executable, or an empty string if * no valid executable path */ -std::string IPAProxy::resolvePath(const std::string &file) const +std::string IPAProxy::resolvePath(std::string_view file) const { - std::string proxyFile = "/" + file; + std::string proxyFile = std::string("/") + file; /* Check env variable first. */ const char *execPaths = utils::secure_getenv("LIBCAMERA_IPA_PROXY_PATH"); diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index d71dad74df70d1fc..34f2a78fcc287066 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -62,7 +62,7 @@ LOG_DEFINE_CATEGORY(MediaDevice) * Once constructed the media device is invalid, and must be populated with * populate() before the media graph can be queried. */ -MediaDevice::MediaDevice(const std::string &deviceNode) +MediaDevice::MediaDevice(std::string_view deviceNode) : deviceNode_(deviceNode), valid_(false), acquired_(false) { } @@ -331,7 +331,7 @@ done: * \param[in] name The entity name * \return The entity with \a name, or nullptr if no such entity is found */ -MediaEntity *MediaDevice::getEntityByName(const std::string &name) const +MediaEntity *MediaDevice::getEntityByName(std::string_view name) const { for (MediaEntity *e : entities_) if (e->name() == name) @@ -359,8 +359,8 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) const * \return The link that connects the two pads, or nullptr if no such a link * exists */ -MediaLink *MediaDevice::link(const std::string &sourceName, unsigned int sourceIdx, - const std::string &sinkName, unsigned int sinkIdx) +MediaLink *MediaDevice::link(std::string_view sourceName, unsigned int sourceIdx, + std::string_view sinkName, unsigned int sinkIdx) { const MediaEntity *source = getEntityByName(sourceName); const MediaEntity *sink = getEntityByName(sinkName); @@ -382,8 +382,8 @@ MediaLink *MediaDevice::link(const std::string &sourceName, unsigned int sourceI * entity \a source, to the pad at index \a sinkIdx of the sink entity \a * sink, if any. * - * \sa link(const std::string &sourceName, unsigned int sourceIdx, - * const std::string &sinkName, unsigned int sinkIdx) + * \sa link(std::string_view sourceName, unsigned int sourceIdx, + * std::string_view sinkName, unsigned int sinkIdx) * \sa link(const MediaPad *source, const MediaPad *sink) * * \return The link that connects the two pads, or nullptr if no such a link @@ -406,8 +406,8 @@ MediaLink *MediaDevice::link(const MediaEntity *source, unsigned int sourceIdx, * \param[in] source The source pad * \param[in] sink The sink pad * - * \sa link(const std::string &sourceName, unsigned int sourceIdx, - * const std::string &sinkName, unsigned int sinkIdx) + * \sa link(std::string_view sourceName, unsigned int sourceIdx, + * std::string_view sinkName, unsigned int sinkIdx) * \sa link(const MediaEntity *source, unsigned int sourceIdx, * const MediaEntity *sink, unsigned int sinkIdx) * diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 8c2c6baf35751d64..51a32316b7008790 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -571,7 +572,7 @@ bool UVCCameraData::generateId() /* Create a controller ID from first device described in firmware. */ std::string controllerId; - std::string searchPath = path; + std::string_view searchPath{ path }; while (true) { std::string::size_type pos = searchPath.rfind('/'); if (pos <= 1) { diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index caa5c20e74836956..4cf9e8256503efac 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -595,8 +595,8 @@ void PipelineHandler::cancelRequest(Request *request) * \return The full path to the pipeline handler configuration file, or an empty * string if no configuration file can be found */ -std::string PipelineHandler::configurationFile(const std::string &subdir, - const std::string &name) const +std::string PipelineHandler::configurationFile(std::string_view subdir, + std::string_view name) const { std::string confPath; struct stat statbuf; @@ -852,7 +852,7 @@ std::vector &PipelineHandlerFactoryBase::factories * \param[in] name The pipeline handler name * \return The factory of the pipeline with name \a name, or nullptr if not found */ -const PipelineHandlerFactoryBase *PipelineHandlerFactoryBase::getFactoryByName(const std::string &name) +const PipelineHandlerFactoryBase *PipelineHandlerFactoryBase::getFactoryByName(std::string_view name) { const std::vector &factories = PipelineHandlerFactoryBase::factories(); diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp index 2b06c5a1a5d15abd..4283cacfa5e0b44a 100644 --- a/src/libcamera/sensor/camera_sensor_properties.cpp +++ b/src/libcamera/sensor/camera_sensor_properties.cpp @@ -78,9 +78,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) * \return A pointer to the CameraSensorProperties instance associated with a sensor * or nullptr if the sensor is not supported */ -const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor) +const CameraSensorProperties *CameraSensorProperties::get(std::string_view sensor) { - static const std::map sensorProps = { + static const std::map sensorProps = { { "ar0144", { .unitCellSize = { 3000, 3000 }, .testPatternModes = { diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 2bea64d9f9947e09..5ca215ed4d5c6abc 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -148,7 +149,7 @@ SoftwareIsp::~SoftwareIsp() } /** - * \fn int SoftwareIsp::loadConfiguration([[maybe_unused]] const std::string &filename) + * \fn int SoftwareIsp::loadConfiguration([[maybe_unused]] std::string_view filename) * \brief Load a configuration from a file * \param[in] filename The file to load the configuration data from * diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp index cbde72d86fd62b41..c1e2b0494e35d6bb 100644 --- a/src/libcamera/sysfs.cpp +++ b/src/libcamera/sysfs.cpp @@ -15,6 +15,8 @@ #include #include +using namespace std::literals::string_literals; + /** * \file sysfs.h * \brief Miscellaneous utility functions to access sysfs @@ -67,13 +69,13 @@ std::string charDevPath(const std::string &deviceNode) * * \return The firmware node path on success or an empty string on failure */ -std::string firmwareNodePath(const std::string &device) +std::string firmwareNodePath(std::string_view device) { std::string fwPath, node; struct stat st; /* Lookup for DT-based systems */ - node = device + "/of_node"; + node = device + "/of_node"s; if (!stat(node.c_str(), &st)) { char *ofPath = realpath(node.c_str(), nullptr); if (!ofPath) @@ -91,7 +93,7 @@ std::string firmwareNodePath(const std::string &device) } /* Lookup for ACPI-based systems */ - node = device + "/firmware_node/path"; + node = device + "/firmware_node/path"s; if (File::exists(node.c_str())) { std::ifstream file(node); if (!file.is_open()) diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 664b74afe6a12757..a8eda2d84357d355 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -56,7 +56,7 @@ LOG_DEFINE_CATEGORY(V4L2) * Initialize the file descriptor to -1 and store the \a deviceNode to be used * at open() time, and the \a logTag to prefix log messages with. */ -V4L2Device::V4L2Device(const std::string &deviceNode) +V4L2Device::V4L2Device(std::string_view deviceNode) : deviceNode_(deviceNode), fdEventNotifier_(nullptr), frameStartEnabled_(false) { diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index 3a0d075f93c457d1..fece69e575b718e1 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -1687,8 +1687,7 @@ const std::string &V4L2Subdevice::model() * \return A newly created V4L2Subdevice on success, nullptr otherwise */ std::unique_ptr -V4L2Subdevice::fromEntityName(const MediaDevice *media, - const std::string &entity) +V4L2Subdevice::fromEntityName(const MediaDevice *media, std::string_view entity) { MediaEntity *mediaEntity = media->getEntityByName(entity); if (!mediaEntity) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a5cf67845ce3600b..91891ffb1590d5b0 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -531,7 +531,7 @@ std::ostream &operator<<(std::ostream &out, const V4L2DeviceFormat &f) * \brief Construct a V4L2VideoDevice * \param[in] deviceNode The file-system path to the video device node */ -V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) +V4L2VideoDevice::V4L2VideoDevice(std::string_view deviceNode) : V4L2Device(deviceNode), formatInfo_(nullptr), cache_(nullptr), fdBufferNotifier_(nullptr), state_(State::Stopped), watchdogDuration_(0.0) @@ -2076,8 +2076,7 @@ void V4L2VideoDevice::watchdogExpired() * \return A newly created V4L2VideoDevice on success, nullptr otherwise */ std::unique_ptr -V4L2VideoDevice::fromEntityName(const MediaDevice *media, - const std::string &entity) +V4L2VideoDevice::fromEntityName(const MediaDevice *media, std::string_view entity) { MediaEntity *mediaEntity = media->getEntityByName(entity); if (!mediaEntity) @@ -2160,7 +2159,7 @@ V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelForma * \brief Create a new V4L2M2MDevice from the \a deviceNode * \param[in] deviceNode The file-system path to the video device node */ -V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode) +V4L2M2MDevice::V4L2M2MDevice(std::string_view deviceNode) : deviceNode_(deviceNode) { output_ = new V4L2VideoDevice(deviceNode); From patchwork Sun Dec 15 23:02:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22327 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 7D6C2C32F6 for ; Sun, 15 Dec 2024 23:02:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 89DFB67F47; Mon, 16 Dec 2024 00:02:41 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="fKLOEEVt"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 613BE67F38 for ; Mon, 16 Dec 2024 00:02:32 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F18C2C6; Mon, 16 Dec 2024 00:01:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303716; bh=oCiYSbfCI97Mme8FpH/9YtMqHWwcNxZWhtoO3dbgo3o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fKLOEEVtTjQmxfb24TbaUks3n6OqzfYmGmtPSlPqCItByWZOzvBK8ErMBoQ1iHcQq z97QRxDisoCehHBhFd3biTHcLwkpML0X1djHyPkYhwfcR/V+HYfjRInjuYUgA/OkQv XZkyF+HEGq5AZRJ0QeCtgOl0v4gEFJSuE/ZYFuAU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 7/8] [DNI] libcamera: Use std::string_view in controls Date: Mon, 16 Dec 2024 01:02:05 +0200 Message-ID: <20241215230206.11002-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" This showcases potential code improvements by using std::string_view in control-related classes. As the std::string_view usage guidelines forbid its usage in the libcamera public API, this patch must not be integrated. Signed-off-by: Laurent Pinchart --- include/libcamera/control_ids.h.in | 3 ++- include/libcamera/controls.h | 21 +++++++++++---------- src/apps/cam/capture_script.h | 3 ++- src/gstreamer/gstlibcamera-controls.cpp.in | 21 ++++++++++++--------- src/libcamera/control_ids.cpp.in | 4 ++-- src/libcamera/controls.cpp | 15 +++++++-------- 6 files changed, 36 insertions(+), 31 deletions(-) diff --git a/include/libcamera/control_ids.h.in b/include/libcamera/control_ids.h.in index 5d0594c687f8eb01..5317368839ffcfc7 100644 --- a/include/libcamera/control_ids.h.in +++ b/include/libcamera/control_ids.h.in @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -46,7 +47,7 @@ enum {{ctrl.name}}Enum { {%- endfor %} }; extern const std::array {{ctrl.name}}Values; -extern const std::map {{ctrl.name}}NameValueMap; +extern const std::map {{ctrl.name}}NameValueMap; {% endif -%} extern const Control<{{ctrl.type}}> {{ctrl.name}}; {% endfor -%} diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index b24336cc280f51db..636f5b980b688e16 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -249,28 +250,28 @@ private: class ControlId { public: - ControlId(unsigned int id, const std::string &name, const std::string &vendor, + ControlId(unsigned int id, std::string_view name, std::string_view vendor, ControlType type, std::size_t size = 0, - const std::map &enumStrMap = {}); + const std::map &enumStrMap = {}); unsigned int id() const { return id_; } - const std::string &name() const { return name_; } - const std::string &vendor() const { return vendor_; } + std::string_view name() const { return name_; } + std::string_view vendor() const { return vendor_; } ControlType type() const { return type_; } bool isArray() const { return size_ > 0; } std::size_t size() const { return size_; } - const std::map &enumerators() const { return reverseMap_; } + const std::map &enumerators() const { return reverseMap_; } private: LIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId) unsigned int id_; - std::string name_; - std::string vendor_; + std::string_view name_; + std::string_view vendor_; ControlType type_; std::size_t size_; - std::map enumStrMap_; - std::map reverseMap_; + std::map enumStrMap_; + std::map reverseMap_; }; static inline bool operator==(unsigned int lhs, const ControlId &rhs) @@ -300,7 +301,7 @@ public: using type = T; Control(unsigned int id, const char *name, const char *vendor, - const std::map &enumStrMap = {}) + const std::map &enumStrMap = {}) : ControlId(id, name, vendor, details::control_type>::value, details::control_type>::size, enumStrMap) { diff --git a/src/apps/cam/capture_script.h b/src/apps/cam/capture_script.h index 294b920363bae01b..254768c259b9e246 100644 --- a/src/apps/cam/capture_script.h +++ b/src/apps/cam/capture_script.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -36,7 +37,7 @@ private: }; using EventPtr = std::unique_ptr; - std::map controls_; + std::map controls_; std::map frameControls_; std::shared_ptr camera_; yaml_parser_t parser_; diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in index ace36b7106867736..31ca6462748d17a1 100644 --- a/src/gstreamer/gstlibcamera-controls.cpp.in +++ b/src/gstreamer/gstlibcamera-controls.cpp.in @@ -154,9 +154,9 @@ bool GstCameraControls::getProperty(guint propId, GValue *value, [[maybe_unused]] GParamSpec *pspec) { if (!controls_acc_.contains(propId)) { - GST_WARNING("Control '%s' is not available, default value will " - "be returned", - controls::controls.at(propId)->name().c_str()); + const std::string_view name = controls::controls.at(propId)->name(); + GST_WARNING("Control '%.*s' is not available, default value will be returned", + static_cast(name.size()), name.data()); return true; } const ControlValue &cv = controls_acc_.get(propId); @@ -211,9 +211,10 @@ bool GstCameraControls::setProperty(guint propId, const GValue *value, auto info = capabilities_.find(cid); if (info == capabilities_.end()) { - GST_WARNING("Control '%s' is not supported by the " + const std::string_view name = cid->name(); + GST_WARNING("Control '%.*s' is not supported by the " "camera and will be ignored", - cid->name().c_str()); + static_cast(name.size()), name.data()); return true; } } @@ -307,12 +308,14 @@ void GstCameraControls::setCamera(const std::shared_ptr &cam) auto info = capabilities_.find(cid); /* Only add controls which are supported. */ - if (info != capabilities_.end()) + if (info != capabilities_.end()) { new_controls.set(id, value); - else - GST_WARNING("Control '%s' is not supported by the " + } else { + const std::string_view name = cid->name(); + GST_WARNING("Control '%.*s' is not supported by the " "camera and will be ignored", - cid->name().c_str()); + static_cast(name.size()), name.data()); + } } controls_acc_ = new_controls; diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index afe9e2c9661041f6..caa1cee165d5c99b 100644 --- a/src/libcamera/control_ids.cpp.in +++ b/src/libcamera/control_ids.cpp.in @@ -51,7 +51,7 @@ namespace {{vendor}} { /** * \var {{ctrl.name}}NameValueMap - * \brief Map of all {{ctrl.name}} supported value names (in std::string format) to value + * \brief Map of all {{ctrl.name}} supported value names (in std::string_view format) to value */ {% endif -%} @@ -84,7 +84,7 @@ extern const std::array {{ctrl.n static_cast<{{ctrl.type}}>({{enum.name}}), {%- endfor %} }; -extern const std::map {{ctrl.name}}NameValueMap = { +extern const std::map {{ctrl.name}}NameValueMap = { {%- for enum in ctrl.enum_values %} { "{{enum.name}}", {{enum.name}} }, {%- endfor %} diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 65eeef2db9c2398f..8c1e61b23c2abcfd 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -406,7 +406,6 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen */ /** - * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type) * \brief Construct a ControlId instance * \param[in] id The control numerical ID * \param[in] name The control name @@ -415,10 +414,10 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \param[in] size The size of the array control, or 0 if scalar control * \param[in] enumStrMap The map from enum names to values (optional) */ -ControlId::ControlId(unsigned int id, const std::string &name, - const std::string &vendor, ControlType type, +ControlId::ControlId(unsigned int id, std::string_view name, + std::string_view vendor, ControlType type, std::size_t size, - const std::map &enumStrMap) + const std::map &enumStrMap) : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), enumStrMap_(enumStrMap) { @@ -433,15 +432,15 @@ ControlId::ControlId(unsigned int id, const std::string &name, */ /** - * \fn const char *ControlId::name() const + * \fn std::string_view ControlId::name() const * \brief Retrieve the control name * \return The control name */ /** - * \fn const std::string &ControlId::vendor() const + * \fn std::string_view ControlId::vendor() const * \brief Retrieve the vendor name - * \return The vendor name, as a string + * \return The vendor name */ /** @@ -464,7 +463,7 @@ ControlId::ControlId(unsigned int id, const std::string &name, */ /** - * \fn const std::map &ControlId::enumerators() const + * \fn const std::map &ControlId::enumerators() const * \brief Retrieve the map of enum values to enum names * \return The map of enum values to enum names */ From patchwork Sun Dec 15 23:02:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 22328 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 39117C32F6 for ; Sun, 15 Dec 2024 23:02:44 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AA67167F42; Mon, 16 Dec 2024 00:02:43 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="n8rDwWmn"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F23FD67F3E for ; Mon, 16 Dec 2024 00:02:33 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 87526675; Mon, 16 Dec 2024 00:01:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1734303717; bh=WL0OPnJd9jT8Vsm8rOUCoVGxIv9BPx2yrJ0DhVBYsg4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=n8rDwWmn1VVFb6650x6VRC3skFIz0p6mC9oAgMD0JQdhjoC9Uwb2rgRnfwk9HcvNn kk91Y5PHLe+rJPYmoM4uEYgaVBrUqs5Ti0dm9kf8N7xBHHLYk2WvIVQCClXK2Mld2W GD+fmOhZ/22FWWmvCXwPyz981DojOh6U17VYMF/A= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Cc: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= Subject: [RFC PATCH 8/8] [DNI] libcamera: yaml_parser: Replace std::string reference with std::string_view Date: Mon, 16 Dec 2024 01:02:06 +0200 Message-ID: <20241215230206.11002-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> References: <20241215230206.11002-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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" This showcases how std::string_view can replace a const std::string reference as a key in a std::map. The patch does not compile due the lack of heterogenous lookup functions for containers (see https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2363r5.html), and must therefore not be integrated. Signed-off-by: Laurent Pinchart --- include/libcamera/internal/yaml_parser.h | 2 +- src/libcamera/yaml_parser.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h index 8c79165659469f65..b92c958f0ec59df4 100644 --- a/include/libcamera/internal/yaml_parser.h +++ b/include/libcamera/internal/yaml_parser.h @@ -124,7 +124,7 @@ public: class DictIterator : public Iterator { public: - using value_type = std::pair; + using value_type = std::pair; using pointer = value_type *; using reference = value_type &; diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index 5ca5fb8235b5e761..555ad03c8dee3f3a 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -312,7 +312,7 @@ template std::optional> YamlObject::getList() const; * iteration order is not specified. * * The iterator's value_type is a - * std::pair. + * std::pair. * * If the YamlObject is not of Dictionary type, the returned adapter operates * as an empty container.