Patch Detail
Show a patch.
GET /api/patches/22321/?format=api
{ "id": 22321, "url": "https://patchwork.libcamera.org/api/patches/22321/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22321/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20241215230206.11002-2-laurent.pinchart@ideasonboard.com>", "date": "2024-12-15T23:01:59", "name": "[RFC,1/8] Documentation: coding-style: Document usage of classes for strings", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "88d533d0d04ef3173839491d69f43b98eec3d95f", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22321/mbox/", "series": [ { "id": 4889, "url": "https://patchwork.libcamera.org/api/series/4889/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4889", "date": "2024-12-15T23:01:58", "name": "libcamera: Use std::string_view", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4889/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22321/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22321/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 DE499C32F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 15 Dec 2024 23:02:29 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8689467F33;\n\tMon, 16 Dec 2024 00:02:27 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2184F67F1D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 16 Dec 2024 00:02:24 +0100 (CET)", "from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D8E502C6;\n\tMon, 16 Dec 2024 00:01:47 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"AQn5DpH8\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734303708;\n\tbh=tl24tobVxMQFnd3EKwnlIOCXTZiAunsUHKJ9v+NMni8=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=AQn5DpH8NF8pwcBoEeo4b6gE843BxbQrNOKRvjLBE6F/YFREVT/mKL/SqUk3E+XIw\n\t/P+62v27JG0QkZoIxk2rIytmx2GsjXz9fwexG5Uatvv4XryrxefyPCAD5ah3DmchXN\n\trmMcqV2iGoPBjS9JfSjuQYvitdsDqh95TYBYQbJU=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>", "Subject": "[RFC PATCH 1/8] Documentation: coding-style: Document usage of\n\tclasses 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", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "C++ has three main ways to handle strings: std::string, std::string_view\nand C-style strings. Document their pros and cons and the rules\ngoverning their usage in libcamera.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n Documentation/coding-style.rst | 98 ++++++++++++++++++++++++++++++++++\n 1 file changed, 98 insertions(+)", "diff": "diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\nindex 6ac3a4a0d5175118..dc4b093b58a99006 100644\n--- a/Documentation/coding-style.rst\n+++ b/Documentation/coding-style.rst\n@@ -269,6 +269,104 @@ the compiler select the right function. This avoids potential errors such as\n calling abs(int) with a float argument, performing an unwanted implicit integer\n conversion. For this reason, cmath is preferred over math.h.\n \n+Strings\n+~~~~~~~\n+\n+This section focusses on strings as a sequence of characters represented by the\n+`char` type, as those are the only strings used in libcamera. The principles\n+are however equally applicable to other types of strings.\n+\n+C++ includes multiple standard ways to represent and handle strings. Each of\n+them have pros and cons and different usage patterns.\n+\n+1. C-style strings\n+\n+ C-style strings are null-terminated arrays of characters of type `char`.\n+ They are represented by a `char *` pointing to the first character. C string\n+ literals (`\"Hello world!\"`) produce read-only global null-terminated arrays\n+ of type `const char`.\n+\n+ Handling strings through `char *` relies on assumptions that are not\n+ enforced at compile time: the memory must not be freed as long as the\n+ pointer remains valid, and the string must be null-terminated. This causes a\n+ risk of use-after-free or buffer out-of-bounds reads. Furthermore, as the\n+ size of the underlying memory is not bundled with the pointer, handling\n+ writable strings as a `char *` risks buffer out-of-bounds writes.\n+\n+2. `std::string` class\n+\n+ The `std::string` class is the main C++ data type to represent strings. The\n+ class holds the string data as a null-terminated array of characters, as\n+ well as the length of the array. `std::string` literals (`\"Hello world!\"s`)\n+ converts the character array literal into a temporary `std::string` instance\n+ whose lifetime ends with the statement.\n+\n+ Usage of `std::string` makes string handling safer and convenient thanks to\n+ the integration with the rest of the C++ standard library. This comes at a\n+ cost, as making copies of the class copies the underlying data, requiring\n+ dynamic allocation of memory. This is partially offset by usage of move\n+ constructors or assignment operators.\n+\n+3. `std::string_view` class\n+\n+ This newer addition to the C++ standard library is a non-owning reference to\n+ a characters array. Like the `std::string` class, the `std::string_view`\n+ stores a pointer to the array and a length, but unlike C-style strings and\n+ the `std::string` class, the array is not guaranteed to be null-terminated.\n+ `std::string_view` literals (`\"Hello world!\"sv`) create a temporary\n+ `std::string_view` instance whose lifetime ends with the statement, but the\n+ underlying character array has global static storage and lifetime.\n+\n+ String views are useful to represent in a single way strings with\n+ heterogenous underlying storage, as they can be constructed from a `char *`\n+ or a `std::string`. They reduce the risk of buffer out-of-bounds accesses as\n+ they carry the array length, and they speed up handling of substrings as\n+ they don't cause copies, unlike `std::string`. As the string views store a\n+ borrowed reference to the underlying storage, they are prone to\n+ use-after-free issues. The lack of a null terminator furthermore makes them\n+ unsafe to handle strings that need to be passed to C functions that assume\n+ null-terminated strings (such as most of the C standard library functions).\n+\n+ C++17 lacks many functions that make `std::string_view` usage convenient.\n+ This includes `operator+()` to concatenate a `std::string` and a\n+ `std::string_view`, as well as heterogenous lookup functions for containers\n+ (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2363r5.html).\n+ libcamera works around the former by defining the missing operators, and the\n+ latter by using the `find()` function instead of `at()` or `operator[]()`.\n+ Those issues are properly addressed in C++26.\n+\n+With those pros, cons are caveats in mind, libcamera follows a set of rules\n+that governs selection of appropriate string types for function arguments:\n+\n+* If the function is internal to a compilation unit, and all callers use the\n+ same string type for the function argument, use that type (by reference or\n+ value, as appropriate).\n+* If the function needs to modify the string, make a copy, or convert it to a\n+ `std::string` for any other reason, pass a `std::string` by value. Use\n+ `std::move()` in the caller if the string is not needed after the function\n+ call, as well as inside the function to move the string to a local copy if\n+ needed. This minimizes the number of copies in all use cases.\n+* If the function needs to pass the string to a C function that takes a\n+ `const char *`, and the caller may reasonably use a C string literal, pass a\n+ `const char *`. This avoids copies when the caller uses a `char *` pointer or\n+ a C string literal, and avoids buffer out-of-bound reads that could occur with\n+ a non null-terminated `std::string_view`.\n+* If the function only needs to pass the string to other functions that take a\n+ `const std::string &` reference, pass a `const std::string &`. This minimizes\n+ the construction of `std::string` instances when the caller already holds an\n+ instance.\n+* If the function only reads from the string, doesn't rely on it being\n+ null-terminated, and can reasonably be called with either C strings, C string\n+ literals, or `std::string` instances, pass a `std::string_view` instance. This\n+ improves safety of the code without impacting performance.\n+* Do not use `std::string_view` in public API function parameters. Previous\n+ rules showed that selection of the optimal string type for function\n+ parameters depends on the internal implementation of the function, which\n+ could change over time. This conflicts with the stability requirements of the\n+ libcamera public API. Furthermore, as C++17 makes `std::string_view` usage\n+ inconvenient in many cases, we don't want to force that inconvenience upon\n+ libcamera users.\n+\n \n Documentation\n -------------\n", "prefixes": [ "RFC", "1/8" ] }