Patch Detail
Show a patch.
GET /api/1.1/patches/20718/?format=api
{ "id": 20718, "url": "https://patchwork.libcamera.org/api/1.1/patches/20718/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20718/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>", "date": "2024-07-29T18:24:44", "name": "[v2] libcamera: Avoid variable-length arrays", "commit_ref": "7f33dfc100b2da0f158494534138a2d8b4f95100", "pull_url": null, "state": "accepted", "archived": false, "hash": "3c98315038cab826e5a948a2a51b5e943a172281", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20718/mbox/", "series": [ { "id": 4466, "url": "https://patchwork.libcamera.org/api/1.1/series/4466/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4466", "date": "2024-07-29T18:24:44", "name": "[v2] libcamera: Avoid variable-length arrays", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4466/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20718/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20718/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 9AA9FBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 29 Jul 2024 18:25:07 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A0A5863370;\n\tMon, 29 Jul 2024 20:25:06 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16E4E61984\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Jul 2024 20:25:05 +0200 (CEST)", "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 58E9245A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Jul 2024 20:24:18 +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=\"F+rGa8HM\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722277458;\n\tbh=MdTNtz0dvOMfaCjxbvrJaivhVsDApyy2WNByhywvW7w=;\n\th=From:To:Subject:Date:From;\n\tb=F+rGa8HMdLrikadV5CjLXUOfKkdppUDI64GNxBBxNHo36+Bwjux7poR7XsJXfGobW\n\th3Vc5n1jZQhsBKimwzMvw49YoW/hO5yux5nGOnXU7hGHFYF+H/tW1sLtXBh3sqze9z\n\t/zPJ1HaPOKxb6h86wkUcb+ZmO2r/PHIMnIfv1GWo=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH v2] libcamera: Avoid variable-length arrays", "Date": "Mon, 29 Jul 2024 21:24:44 +0300", "Message-ID": "<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.44.2", "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": "Unlike in C where they have been standardized since C99, variable-length\narrays in C++ are an extension supported by gcc and clang. Clang started\nwarning about this in version 18:\n\nsrc/libcamera/ipc_unixsocket.cpp:250:11: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n 250 | char buf[CMSG_SPACE(num * sizeof(uint32_t))];\n | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nOne simple option is to disable the warning. However, usage of VLAs in\nC++ is discouraged by some, usually due to security reasons, based on\nthe rationale that developers are often unaware of unintentional use of\nVLAs and how they may affect the security of the code when the array\nsize is not properly validated.\n\nThis rationale may sound dubious, as the most commonly proposed fix is\nto replace VLAs with vectors (or just arrays dynamically allocated with\nnew() wrapped in unique pointers), without adding any size validation.\nThis will not produce much better results. However, keeping the VLA\nwarning and converting the code to dynamic allocation may still be\nslightly better, as it can prompt developers to notice VLAs and check if\nsize validation is required.\n\nFor these reasons, convert all VLAs to std::vector. Most of the VLAs\ndon't need extra size validation, as the size is bound through different\nconstraints (e.g. image width for line buffers). An arguable exception\nmay be the buffers in IPCUnixSocket::sendData() and\nIPCUnixSocket::recvData() as the number of fds is not bound-checked\nlocally, but we will run out of file descriptors before we could\noverflow the buffer size calculation.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v1:\n\n- Fix argument to read() in IPC unix socket test\n---\n src/android/jpeg/encoder_libjpeg.cpp | 6 +++---\n src/apps/common/dng_writer.cpp | 11 ++++++-----\n src/apps/common/options.cpp | 8 +++++---\n src/ipa/rpi/controller/rpi/alsc.cpp | 6 ++++--\n src/libcamera/ipc_unixsocket.cpp | 11 +++++------\n test/ipc/unixsocket.cpp | 7 ++++---\n 6 files changed, 27 insertions(+), 22 deletions(-)\n\n\nbase-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9", "diff": "diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\nindex 7fc6287e4bdb..cb242b5ec6a8 100644\n--- a/src/android/jpeg/encoder_libjpeg.cpp\n+++ b/src/android/jpeg/encoder_libjpeg.cpp\n@@ -125,7 +125,7 @@ void EncoderLibJpeg::compressRGB(const std::vector<Span<uint8_t>> &planes)\n */\n void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)\n {\n-\tuint8_t tmprowbuf[compress_.image_width * 3];\n+\tstd::vector<uint8_t> tmprowbuf(compress_.image_width * 3);\n \n \t/*\n \t * \\todo Use the raw api, and only unpack the cb/cr samples to new line\n@@ -149,10 +149,10 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)\n \tconst unsigned char *src_c = planes[1].data();\n \n \tJSAMPROW row_pointer[1];\n-\trow_pointer[0] = &tmprowbuf[0];\n+\trow_pointer[0] = tmprowbuf.data();\n \n \tfor (unsigned int y = 0; y < compress_.image_height; y++) {\n-\t\tunsigned char *dst = &tmprowbuf[0];\n+\t\tunsigned char *dst = tmprowbuf.data();\n \n \t\tconst unsigned char *src_y = src + y * y_stride;\n \t\tconst unsigned char *src_cb = src_c + (y / vertSubSample) * c_stride + cb_pos;\ndiff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\nindex 355433b08d68..ac4619511677 100644\n--- a/src/apps/common/dng_writer.cpp\n+++ b/src/apps/common/dng_writer.cpp\n@@ -11,6 +11,7 @@\n #include <endian.h>\n #include <iostream>\n #include <map>\n+#include <vector>\n \n #include <tiffio.h>\n \n@@ -544,7 +545,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n \t * or a thumbnail scanline. The latter will always be much smaller than\n \t * the former as we downscale by 16 in both directions.\n \t */\n-\tuint8_t scanline[(config.size.width * info->bitsPerSample + 7) / 8];\n+\tstd::vector<uint8_t> scanline((config.size.width * info->bitsPerSample + 7) / 8);\n \n \ttoff_t rawIFDOffset = 0;\n \ttoff_t exifIFDOffset = 0;\n@@ -644,10 +645,10 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n \t/* Write the thumbnail. */\n \tconst uint8_t *row = static_cast<const uint8_t *>(data);\n \tfor (unsigned int y = 0; y < config.size.height / 16; y++) {\n-\t\tinfo->thumbScanline(*info, &scanline, row,\n+\t\tinfo->thumbScanline(*info, scanline.data(), row,\n \t\t\t\t config.size.width / 16, config.stride);\n \n-\t\tif (TIFFWriteScanline(tif, &scanline, y, 0) != 1) {\n+\t\tif (TIFFWriteScanline(tif, scanline.data(), y, 0) != 1) {\n \t\t\tstd::cerr << \"Failed to write thumbnail scanline\"\n \t\t\t\t << std::endl;\n \t\t\tTIFFClose(tif);\n@@ -747,9 +748,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n \t/* Write RAW content. */\n \trow = static_cast<const uint8_t *>(data);\n \tfor (unsigned int y = 0; y < config.size.height; y++) {\n-\t\tinfo->packScanline(&scanline, row, config.size.width);\n+\t\tinfo->packScanline(scanline.data(), row, config.size.width);\n \n-\t\tif (TIFFWriteScanline(tif, &scanline, y, 0) != 1) {\n+\t\tif (TIFFWriteScanline(tif, scanline.data(), y, 0) != 1) {\n \t\t\tstd::cerr << \"Failed to write RAW scanline\"\n \t\t\t\t << std::endl;\n \t\t\tTIFFClose(tif);\ndiff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\nindex ab19aa3d48e7..ece268d0e344 100644\n--- a/src/apps/common/options.cpp\n+++ b/src/apps/common/options.cpp\n@@ -10,6 +10,7 @@\n #include <iomanip>\n #include <iostream>\n #include <string.h>\n+#include <vector>\n \n #include \"options.h\"\n \n@@ -879,8 +880,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n \t * Allocate short and long options arrays large enough to contain all\n \t * options.\n \t */\n-\tchar shortOptions[optionsMap_.size() * 3 + 2];\n-\tstruct option longOptions[optionsMap_.size() + 1];\n+\tstd::vector<char> shortOptions(optionsMap_.size() * 3 + 2);\n+\tstd::vector<struct option> longOptions(optionsMap_.size() + 1);\n \tunsigned int ids = 0;\n \tunsigned int idl = 0;\n \n@@ -922,7 +923,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n \topterr = 0;\n \n \twhile (true) {\n-\t\tint c = getopt_long(argc, argv, shortOptions, longOptions, nullptr);\n+\t\tint c = getopt_long(argc, argv, shortOptions.data(),\n+\t\t\t\t longOptions.data(), nullptr);\n \n \t\tif (c == -1)\n \t\t\tbreak;\ndiff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\nindex 67029fc34d6a..161fd45526ec 100644\n--- a/src/ipa/rpi/controller/rpi/alsc.cpp\n+++ b/src/ipa/rpi/controller/rpi/alsc.cpp\n@@ -9,6 +9,7 @@\n #include <functional>\n #include <math.h>\n #include <numeric>\n+#include <vector>\n \n #include <libcamera/base/log.h>\n #include <libcamera/base/span.h>\n@@ -496,8 +497,9 @@ void resampleCalTable(const Array2D<double> &calTableIn,\n \t * Precalculate and cache the x sampling locations and phases to save\n \t * recomputing them on every row.\n \t */\n-\tint xLo[X], xHi[X];\n-\tdouble xf[X];\n+\tstd::vector<int> xLo(X);\n+\tstd::vector<int> xHi(X);\n+\tstd::vector<double> xf(X);\n \tdouble scaleX = cameraMode.sensorWidth /\n \t\t\t(cameraMode.width * cameraMode.scaleX);\n \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\ndiff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\nindex 75285b679eac..002053e35557 100644\n--- a/src/libcamera/ipc_unixsocket.cpp\n+++ b/src/libcamera/ipc_unixsocket.cpp\n@@ -12,6 +12,7 @@\n #include <string.h>\n #include <sys/socket.h>\n #include <unistd.h>\n+#include <vector>\n \n #include <libcamera/base/event_notifier.h>\n #include <libcamera/base/log.h>\n@@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n \tiov[0].iov_base = const_cast<void *>(buffer);\n \tiov[0].iov_len = length;\n \n-\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n-\tmemset(buf, 0, sizeof(buf));\n+\tstd::vector<uint8_t> buf(CMSG_SPACE(num * sizeof(uint32_t)));\n \n-\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n+\tstruct cmsghdr *cmsg = reinterpret_cast<struct cmsghdr *>(buf.data());\n \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n \tcmsg->cmsg_level = SOL_SOCKET;\n \tcmsg->cmsg_type = SCM_RIGHTS;\n@@ -283,10 +283,9 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n \tiov[0].iov_base = buffer;\n \tiov[0].iov_len = length;\n \n-\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n-\tmemset(buf, 0, sizeof(buf));\n+\tstd::vector<uint8_t> buf(CMSG_SPACE(num * sizeof(uint32_t)));\n \n-\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n+\tstruct cmsghdr *cmsg = reinterpret_cast<struct cmsghdr *>(buf.data());\n \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n \tcmsg->cmsg_level = SOL_SOCKET;\n \tcmsg->cmsg_type = SCM_RIGHTS;\ndiff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\nindex 2546882da085..f39bd986b8ae 100644\n--- a/test/ipc/unixsocket.cpp\n+++ b/test/ipc/unixsocket.cpp\n@@ -15,6 +15,7 @@\n #include <sys/types.h>\n #include <sys/wait.h>\n #include <unistd.h>\n+#include <vector>\n \n #include <libcamera/base/event_dispatcher.h>\n #include <libcamera/base/thread.h>\n@@ -340,14 +341,14 @@ protected:\n \n \t\tfor (unsigned int i = 0; i < std::size(strings); i++) {\n \t\t\tunsigned int len = strlen(strings[i]);\n-\t\t\tchar buf[len];\n+\t\t\tstd::vector<char> buf(len);\n \n \t\t\tclose(fds[i]);\n \n-\t\t\tif (read(response.fds[0], &buf, len) <= 0)\n+\t\t\tif (read(response.fds[0], buf.data(), len) <= 0)\n \t\t\t\treturn TestFail;\n \n-\t\t\tif (memcmp(buf, strings[i], len))\n+\t\t\tif (memcmp(buf.data(), strings[i], len))\n \t\t\t\treturn TestFail;\n \t\t}\n \n", "prefixes": [ "v2" ] }