[{"id":30497,"web_url":"https://patchwork.libcamera.org/comment/30497/","msgid":"<871q3be341.fsf@redhat.com>","date":"2024-07-30T07:43:58","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nLaurent Pinchart writes:\n\n> Unlike in C where they have been standardized since C99, variable-length\n> arrays in C++ are an extension supported by gcc and clang. Clang started\n> warning about this in version 18:\n>\n> src/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>\n> One simple option is to disable the warning. However, usage of VLAs in\n> C++ is discouraged by some, usually due to security reasons, based on\n> the rationale that developers are often unaware of unintentional use of\n> VLAs and how they may affect the security of the code when the array\n> size is not properly validated.\n>\n> This rationale may sound dubious, as the most commonly proposed fix is\n> to replace VLAs with vectors (or just arrays dynamically allocated with\n> new() wrapped in unique pointers), without adding any size validation.\n> This will not produce much better results. However, keeping the VLA\n> warning and converting the code to dynamic allocation may still be\n> slightly better, as it can prompt developers to notice VLAs and check if\n> size validation is required.\n>\n> For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> don't need extra size validation, as the size is bound through different\n> constraints (e.g. image width for line buffers). An arguable exception\n> may be the buffers in IPCUnixSocket::sendData() and\n> IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> locally, but we will run out of file descriptors before we could\n> overflow the buffer size calculation.\n\nHmm, your argumentation is convincing and I don't have any better idea\nthan to accept using std::vector::data() in the given places.\n\nReviewed-by: Milan Zamazal \n\n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes 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> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index 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> &planes)\n> */\n> void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> {\n> -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> +\tstd::vector 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> &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;\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index 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 \n> #include \n> #include \n> +#include \n> \n> #include \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 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(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(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);\n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index ab19aa3d48e7..ece268d0e344 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> +\tstd::vector 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;\n> diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> index 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 \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> +\tstd::vector xHi(X);\n> +\tstd::vector xf(X);\n> \tdouble scaleX = cameraMode.sensorWidth /\n> \t\t\t(cameraMode.width * cameraMode.scaleX);\n> \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 75285b679eac..002053e35557 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> \n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> \n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 2546882da085..f39bd986b8ae 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \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 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>\n> base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7E960C323E\n\tfor ;\n\tTue, 30 Jul 2024 07:44:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AAE6563373;\n\tTue, 30 Jul 2024 09:44:08 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7EE996198D\n\tfor ;\n\tTue, 30 Jul 2024 09:44:06 +0200 (CEST)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-221-4ms8qVbHOB-TQ8Gn-L9dBg-1; Tue, 30 Jul 2024 03:44:02 -0400","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-4281b7196bbso17026835e9.0\n\tfor ;\n\tTue, 30 Jul 2024 00:44:02 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-36b367c032bsm14053148f8f.12.2024.07.30.00.43.58\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 30 Jul 2024 00:43:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"aO/NCttb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722325445;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=qESGZodOVn5u9RXZPAULQZemc5W59+CAWizZZK2uACw=;\n\tb=aO/NCttbur1sfeVQGSH42g5nmhVEWuEOlOBpHufr1V7liqxnKGMbor9Z0yNCn1pZfEAMcH\n\ta1ZY+OW92fRrJRNFnsAXvRhaDlNLlWKzc9syTe/GlKbZf1JnNMF7KlbwQl8Y8EjlcsYw4M\n\t+zuMHYRtSxQXTRQ2/v94DcCOm8gELZI=","X-MC-Unique":"4ms8qVbHOB-TQ8Gn-L9dBg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722325440; x=1722930240;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=qESGZodOVn5u9RXZPAULQZemc5W59+CAWizZZK2uACw=;\n\tb=ngoEDi/a85P3O38Mu6XMqtAh2S/wtk196bvtXtEO7YSuYGt2wbaEh20yJoXJLR+ASP\n\tuEiX8fHwDupvwtdDglzu6pcpU3PsJD7sY+H7yZR9iX7yRw597xsg+hXOA0szN27nOnK4\n\tmxBhGq9RwfcZ180mgvrINTpSBsZMfhVDYQzrVcQZ/zncHaJAr3DyQjWtkdg+cH+gNWKB\n\tbiOsBD6B/izfKculGbFCj4NTr2svMu4u3nnPhf5kVE9ik/U/SxyY58BBcgsNFcKiwa/R\n\tYcBhaF1yu9wsn6QuBUBlJmFVZ12uRohQhB/GDLcoUPkIeWGOPKIgIElOs2Y+iDceX3c5\n\t26jw==","X-Gm-Message-State":"AOJu0YyeTcjFRVT2s3FnBaf4Onf/g2LV1vht97Cc7gqrXiGAD3qj4EeI\n\tgbgaBvAtIZiYY6u2zMGzuFi0VlnpRMAMm58v/G3jMsLV3qYALQP91T7FcAaqyuxzkkLQv/OZoeE\n\ttk++ttllhW7ifimYyY/XZ9qPyQDMODsvb4QSv5ECGn6Xo5ziownd3uM1KKn2TXAv7XBI69PKSfX\n\tuC7xhxpNJjbDe+mo9b1N2X/4MYbjUXW15la9eN1pLcsZYPQ6RagBo6uUk=","X-Received":["by 2002:adf:e011:0:b0:367:9903:a81 with SMTP id\n\tffacd0b85a97d-36b5d353904mr7141322f8f.43.1722325440479; \n\tTue, 30 Jul 2024 00:44:00 -0700 (PDT)","by 2002:adf:e011:0:b0:367:9903:a81 with SMTP id\n\tffacd0b85a97d-36b5d353904mr7141305f8f.43.1722325439931; \n\tTue, 30 Jul 2024 00:43:59 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHnLuIYbS9TfEYPz9kZ8PWKUQSvNp3zAtivmjcgJUH4l/CJzOY1fJk+xPBpl4oahYMZdLxCFg==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","In-Reply-To":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Mon, 29 Jul 2024 21:24:44 +0300\")","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>","Date":"Tue, 30 Jul 2024 09:43:58 +0200","Message-ID":"<871q3be341.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30498,"web_url":"https://patchwork.libcamera.org/comment/30498/","msgid":"<20240730110157.GF1552@pendragon.ideasonboard.com>","date":"2024-07-30T11:01:57","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Tue, Jul 30, 2024 at 09:43:58AM +0200, Milan Zamazal wrote:\n> Laurent Pinchart writes:\n> \n> > Unlike in C where they have been standardized since C99, variable-length\n> > arrays in C++ are an extension supported by gcc and clang. Clang started\n> > warning about this in version 18:\n> >\n> > src/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> >\n> > One simple option is to disable the warning. However, usage of VLAs in\n> > C++ is discouraged by some, usually due to security reasons, based on\n> > the rationale that developers are often unaware of unintentional use of\n> > VLAs and how they may affect the security of the code when the array\n> > size is not properly validated.\n> >\n> > This rationale may sound dubious, as the most commonly proposed fix is\n> > to replace VLAs with vectors (or just arrays dynamically allocated with\n> > new() wrapped in unique pointers), without adding any size validation.\n> > This will not produce much better results. However, keeping the VLA\n> > warning and converting the code to dynamic allocation may still be\n> > slightly better, as it can prompt developers to notice VLAs and check if\n> > size validation is required.\n> >\n> > For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> > don't need extra size validation, as the size is bound through different\n> > constraints (e.g. image width for line buffers). An arguable exception\n> > may be the buffers in IPCUnixSocket::sendData() and\n> > IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> > locally, but we will run out of file descriptors before we could\n> > overflow the buffer size calculation.\n> \n> Hmm, your argumentation is convincing and I don't have any better idea\n> than to accept using std::vector::data() in the given places.\n\nI'm not entirely thrilled by this, as I think VLAs are fine in this\ncase, but the dynamic heap allocation with std::vector shouldn't be a\nbig burden in any of the code paths below.\n\n> Reviewed-by: Milan Zamazal \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes 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> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > index 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> &planes)\n> > */\n> > void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> > {\n> > -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> > +\tstd::vector 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> &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;\n> > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> > index 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 \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \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 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(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(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);\n> > diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> > index ab19aa3d48e7..ece268d0e344 100644\n> > --- a/src/apps/common/options.cpp\n> > +++ b/src/apps/common/options.cpp\n> > @@ -10,6 +10,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> > +\tstd::vector 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;\n> > diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> > index 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 \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> > +\tstd::vector xHi(X);\n> > +\tstd::vector xf(X);\n> > \tdouble scaleX = cameraMode.sensorWidth /\n> > \t\t\t(cameraMode.width * cameraMode.scaleX);\n> > \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > index 75285b679eac..002053e35557 100644\n> > --- a/src/libcamera/ipc_unixsocket.cpp\n> > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> > \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > \n> > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > \n> > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tstruct cmsghdr *cmsg = reinterpret_cast(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> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > index 2546882da085..f39bd986b8ae 100644\n> > --- a/test/ipc/unixsocket.cpp\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -15,6 +15,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \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 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> >\n> > base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1DD1FBDC71\n\tfor ;\n\tTue, 30 Jul 2024 11:02:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AED8D63374;\n\tTue, 30 Jul 2024 13:02:19 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 751B761994\n\tfor ;\n\tTue, 30 Jul 2024 13:02:18 +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 27BBD743;\n\tTue, 30 Jul 2024 13:01:31 +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=\"BYwlGd2S\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722337291;\n\tbh=f/Upq0wic3ElvVxSiME/e+92wwUsZ9rYE6Pkzxd7INM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BYwlGd2SXwY2deT5OEWfrXGujpOTMuhuG4mtBG+2ZDTF6Kw3Gs5mGJcxl9tUMvBFq\n\t7lPvCRIkqbEmk5A/O85Z0dUyhsQZkRWwnNla4oEmR1s9j32qeiVHaz7MtIgcpCgbkG\n\tVV8Js/iu1eOafRmnnyUhtELATboRI2D8phE6mAAc=","Date":"Tue, 30 Jul 2024 14:01:57 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","Message-ID":"<20240730110157.GF1552@pendragon.ideasonboard.com>","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>\n\t<871q3be341.fsf@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<871q3be341.fsf@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30500,"web_url":"https://patchwork.libcamera.org/comment/30500/","msgid":"","date":"2024-07-30T12:48:05","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Mon, Jul 29, 2024 at 09:24:44PM GMT, Laurent Pinchart wrote:\n> Unlike in C where they have been standardized since C99, variable-length\n> arrays in C++ are an extension supported by gcc and clang. Clang started\n> warning about this in version 18:\n>\n> src/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>\n> One simple option is to disable the warning. However, usage of VLAs in\n> C++ is discouraged by some, usually due to security reasons, based on\n> the rationale that developers are often unaware of unintentional use of\n> VLAs and how they may affect the security of the code when the array\n> size is not properly validated.\n>\n> This rationale may sound dubious, as the most commonly proposed fix is\n> to replace VLAs with vectors (or just arrays dynamically allocated with\n> new() wrapped in unique pointers), without adding any size validation.\n> This will not produce much better results. However, keeping the VLA\n> warning and converting the code to dynamic allocation may still be\n> slightly better, as it can prompt developers to notice VLAs and check if\n> size validation is required.\n>\n> For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> don't need extra size validation, as the size is bound through different\n> constraints (e.g. image width for line buffers). An arguable exception\n> may be the buffers in IPCUnixSocket::sendData() and\n> IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> locally, but we will run out of file descriptors before we could\n> overflow the buffer size calculation.\n>\n> Signed-off-by: Laurent Pinchart \n\nAcked-by: Jacopo Mondi \n\n> ---\n> Changes 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> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index 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> &planes)\n> */\n> void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> {\n> -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> +\tstd::vector 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> &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;\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index 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 \n> #include \n> #include \n> +#include \n>\n> #include \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 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(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(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);\n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index ab19aa3d48e7..ece268d0e344 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> +\tstd::vector 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;\n> diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> index 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 \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> +\tstd::vector xHi(X);\n> +\tstd::vector xf(X);\n> \tdouble scaleX = cameraMode.sensorWidth /\n> \t\t\t(cameraMode.width * cameraMode.scaleX);\n> \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 75285b679eac..002053e35557 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \n> @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n>\n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n>\n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 2546882da085..f39bd986b8ae 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> #include \n> +#include \n>\n> #include \n> #include \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 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>\n> base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B9DE2BDC71\n\tfor ;\n\tTue, 30 Jul 2024 12:48:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8EFA56336E;\n\tTue, 30 Jul 2024 14:48:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CF3E61994\n\tfor ;\n\tTue, 30 Jul 2024 14:48:09 +0200 (CEST)","from ideasonboard.com (mob-5-90-63-112.net.vodafone.it\n\t[5.90.63.112])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1365E743;\n\tTue, 30 Jul 2024 14:47:22 +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=\"fUUZF9EA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722343642;\n\tbh=6qe/a/7quK9Zgvjh11f+VIiIjn0bZnlRMtKOjR91BK8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fUUZF9EA0P2Feu+ZJYEYza47VKCHQVoXRdfcaDeP/wPM0zAIpyZS3jHvOHWeZH+KV\n\tqXCNVJN1izneHUiwxhYANCmKEpMVw22fvLUU9ybfNSifeliArn7dUCsPZqtAWdCQfS\n\te5VIxTohb+pLXB9LQOf2bReGmNo3VCt/4Ry9OiAw=","Date":"Tue, 30 Jul 2024 14:48:05 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","Message-ID":"","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30503,"web_url":"https://patchwork.libcamera.org/comment/30503/","msgid":"","date":"2024-07-30T15:55:23","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. július 29., hétfő 20:24 keltezéssel, Laurent Pinchart írta:\n\n> Unlike in C where they have been standardized since C99, variable-length\n> arrays in C++ are an extension supported by gcc and clang. Clang started\n> warning about this in version 18:\n> \n> src/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> \n\nAs far as I know the warning has existed for a long time, but it was behind\n`-Wvla-extension` for C++ before clang 18, but that seems to be a part of Wpedantic,\nso it was not enabled during normal libcamera builds.\n\n\n> One simple option is to disable the warning. However, usage of VLAs in\n> C++ is discouraged by some, usually due to security reasons, based on\n> the rationale that developers are often unaware of unintentional use of\n> VLAs and how they may affect the security of the code when the array\n> size is not properly validated.\n> \n> This rationale may sound dubious, as the most commonly proposed fix is\n> to replace VLAs with vectors (or just arrays dynamically allocated with\n> new() wrapped in unique pointers), without adding any size validation.\n> This will not produce much better results. However, keeping the VLA\n> warning and converting the code to dynamic allocation may still be\n> slightly better, as it can prompt developers to notice VLAs and check if\n> size validation is required.\n> \n> For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> don't need extra size validation, as the size is bound through different\n> constraints (e.g. image width for line buffers).\n\nUnfortunately it does not appear applicable here, but where Qt is used,\n`QVarLengthArray` can be a nice alternative.\n\nI believe another alternative that is worth considering is the array version\nof unique_ptr.\n\n\n> An arguable exception\n> may be the buffers in IPCUnixSocket::sendData() and\n> IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> locally, but we will run out of file descriptors before we could\n> overflow the buffer size calculation.\n\nThe maximum number of file descriptors that can be transferred in a single `sendmsg()`\ncall has been 253 for a long time on Linux.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes 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> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index 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> &planes)\n> */\n> void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> {\n> -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> +\tstd::vector 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> &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;\n> diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> index 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 \n> #include \n> #include \n> +#include \n> \n> #include \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 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(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(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);\n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index ab19aa3d48e7..ece268d0e344 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> #include \n> +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> +\tstd::vector 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;\n> diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> index 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 \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> +\tstd::vector xHi(X);\n> +\tstd::vector xf(X);\n> \tdouble scaleX = cameraMode.sensorWidth /\n> \t\t\t(cameraMode.width * cameraMode.scaleX);\n> \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 75285b679eac..002053e35557 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> \n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> \n> -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tstruct cmsghdr *cmsg = reinterpret_cast(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> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> index 2546882da085..f39bd986b8ae 100644\n> --- a/test/ipc/unixsocket.cpp\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \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 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> \n> base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9\n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 173FEC323E\n\tfor ;\n\tTue, 30 Jul 2024 15:55:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23DC363374;\n\tTue, 30 Jul 2024 17:55:31 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BCAF61994\n\tfor ;\n\tTue, 30 Jul 2024 17:55:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"CGIkCIiX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1722354928; x=1722614128;\n\tbh=rDyugchCiiZJLpHgeQgdo079qONwyVI5x2wYG/LTDQ0=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=CGIkCIiXEg9mZ+PKZvchf1YTyBgC9dJzuuPRbD+k4KuR4700WBMkhgzO5vgIEf88b\n\tc/Jd3MrkW0Bm2RBVCNXQ9KmeX4st4thiguu2FzMR2JLuclfYCf+guuCwihTNdJlCJY\n\tw+rV57Zrz771UBnqkpqpm/DuF/tYZxEhgbkftSw4eWlhs104ubICnsvNo2sNaFuxaI\n\t3wy4rpgrBLR/8b3mRmmwRo9m72ltWyGPDKr469+rUE5Qc+aM1XNJ5WBvyBK+TYW0fj\n\tseaDyuMxIl17v1GWa1x4STrIiu1E1hZH6j+M2qBHNvrJn0dQolVpcyEukLOtVDG2QS\n\t1+RBnYxvPx6dQ==","Date":"Tue, 30 Jul 2024 15:55:23 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","Message-ID":"","In-Reply-To":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"891c0981e43e96ffd157c6f3a8c97e5f208f5818","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30506,"web_url":"https://patchwork.libcamera.org/comment/30506/","msgid":"<20240730222546.GI8146@pendragon.ideasonboard.com>","date":"2024-07-30T22:25:46","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jul 30, 2024 at 03:55:23PM +0000, Barnabás Pőcze wrote:\n> 2024. július 29., hétfő 20:24 keltezéssel, Laurent Pinchart írta:\n> \n> > Unlike in C where they have been standardized since C99, variable-length\n> > arrays in C++ are an extension supported by gcc and clang. Clang started\n> > warning about this in version 18:\n> > \n> > src/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> > \n> \n> As far as I know the warning has existed for a long time, but it was behind\n> `-Wvla-extension` for C++ before clang 18, but that seems to be a part of Wpedantic,\n> so it was not enabled during normal libcamera builds.\n\nIndeed. I'll update the commit message to \"about this with -Wall in\nversion 18\".\n\n> > One simple option is to disable the warning. However, usage of VLAs in\n> > C++ is discouraged by some, usually due to security reasons, based on\n> > the rationale that developers are often unaware of unintentional use of\n> > VLAs and how they may affect the security of the code when the array\n> > size is not properly validated.\n> > \n> > This rationale may sound dubious, as the most commonly proposed fix is\n> > to replace VLAs with vectors (or just arrays dynamically allocated with\n> > new() wrapped in unique pointers), without adding any size validation.\n> > This will not produce much better results. However, keeping the VLA\n> > warning and converting the code to dynamic allocation may still be\n> > slightly better, as it can prompt developers to notice VLAs and check if\n> > size validation is required.\n> > \n> > For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> > don't need extra size validation, as the size is bound through different\n> > constraints (e.g. image width for line buffers).\n> \n> Unfortunately it does not appear applicable here, but where Qt is used,\n> `QVarLengthArray` can be a nice alternative.\n> \n> I believe another alternative that is worth considering is the array version\n> of unique_ptr.\n\nDo you mean something like\n\n\tstd::unique_ptr tmprowbuf(new uint8_t[compress_.image_width * 3]);\n\n? I've thought about it, but I don't think it really improves runtime\nperformance much compared to std::vector in the usage patterns below.\n\n> > An arguable exception\n> > may be the buffers in IPCUnixSocket::sendData() and\n> > IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> > locally, but we will run out of file descriptors before we could\n> > overflow the buffer size calculation.\n> \n> The maximum number of file descriptors that can be transferred in a single `sendmsg()`\n> call has been 253 for a long time on Linux.\n\nIndeed, another reason why we should be safe.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes 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> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > index 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> &planes)\n> > */\n> > void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> > {\n> > -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> > +\tstd::vector 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> &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;\n> > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> > index 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 \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \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 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(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(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);\n> > diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> > index ab19aa3d48e7..ece268d0e344 100644\n> > --- a/src/apps/common/options.cpp\n> > +++ b/src/apps/common/options.cpp\n> > @@ -10,6 +10,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> > +\tstd::vector 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;\n> > diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> > index 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 \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> > +\tstd::vector xHi(X);\n> > +\tstd::vector xf(X);\n> > \tdouble scaleX = cameraMode.sensorWidth /\n> > \t\t\t(cameraMode.width * cameraMode.scaleX);\n> > \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > index 75285b679eac..002053e35557 100644\n> > --- a/src/libcamera/ipc_unixsocket.cpp\n> > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> > \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > \n> > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > \n> > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tstruct cmsghdr *cmsg = reinterpret_cast(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> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > index 2546882da085..f39bd986b8ae 100644\n> > --- a/test/ipc/unixsocket.cpp\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -15,6 +15,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \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 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> > \n> > base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BA00EC323E\n\tfor ;\n\tTue, 30 Jul 2024 22:26:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06AED63376;\n\tWed, 31 Jul 2024 00:26:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BDD561984\n\tfor ;\n\tWed, 31 Jul 2024 00:26:07 +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 AE9216EF;\n\tWed, 31 Jul 2024 00:25:19 +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=\"SDFb+t2M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722378319;\n\tbh=goi0yU2KqEIws8ORjmZ3Cxyat800Uy7qS2X9arA7VNQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SDFb+t2M/VuvfaOj7RmZWt/hQbcP+RKBdK4k60Rbttnb6mVeuOcns/2mIyXF5nZNb\n\t+aKhx9LuE4rmgAduMgEnrcnIl6Iw9kufvqRl6Q3hSThycLjhGSpZzoQfd7bQSh9oEw\n\tsuyanIzrdk7QOBO15fJkFR1u7yfzm3CzHZ0ZYxqE=","Date":"Wed, 31 Jul 2024 01:25:46 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","Message-ID":"<20240730222546.GI8146@pendragon.ideasonboard.com>","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30507,"web_url":"https://patchwork.libcamera.org/comment/30507/","msgid":"","date":"2024-07-30T22:39:04","subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. július 31., szerda 0:25 keltezéssel, Laurent Pinchart írta:\n\nOn Tue, Jul 30, 2024 at 03:55:23PM +0000, Barnabás Pőcze wrote:\n> > 2024. július 29., hétfő 20:24 keltezéssel, Laurent Pinchart írta:\n> >\n> > > Unlike in C where they have been standardized since C99, variable-length\n> > > arrays in C++ are an extension supported by gcc and clang. Clang started\n> > > warning about this in version 18:\n> > >\n> > > src/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> > >\n> >\n> > As far as I know the warning has existed for a long time, but it was behind\n> > `-Wvla-extension` for C++ before clang 18, but that seems to be a part of Wpedantic,\n> > so it was not enabled during normal libcamera builds.\n> \n> Indeed. I'll update the commit message to \"about this with -Wall in\n> version 18\".\n> \n> > > One simple option is to disable the warning. However, usage of VLAs in\n> > > C++ is discouraged by some, usually due to security reasons, based on\n> > > the rationale that developers are often unaware of unintentional use of\n> > > VLAs and how they may affect the security of the code when the array\n> > > size is not properly validated.\n> > >\n> > > This rationale may sound dubious, as the most commonly proposed fix is\n> > > to replace VLAs with vectors (or just arrays dynamically allocated with\n> > > new() wrapped in unique pointers), without adding any size validation.\n> > > This will not produce much better results. However, keeping the VLA\n> > > warning and converting the code to dynamic allocation may still be\n> > > slightly better, as it can prompt developers to notice VLAs and check if\n> > > size validation is required.\n> > >\n> > > For these reasons, convert all VLAs to std::vector. Most of the VLAs\n> > > don't need extra size validation, as the size is bound through different\n> > > constraints (e.g. image width for line buffers).\n> >\n> > Unfortunately it does not appear applicable here, but where Qt is used,\n> > `QVarLengthArray` can be a nice alternative.\n> >\n> > I believe another alternative that is worth considering is the array version\n> > of unique_ptr.\n> \n> Do you mean something like\n> \n> \tstd::unique_ptr tmprowbuf(new uint8_t[compress_.image_width * 3]);\n> \n> ? I've thought about it, but I don't think it really improves runtime\n> performance much compared to std::vector in the usage patterns below.\n\nYes, although my preferred form is\n\n auto p = std::make_unique(n);\n\nand yes, I also don't think it changes the performance characteristic too much.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> > > An arguable exception\n> > > may be the buffers in IPCUnixSocket::sendData() and\n> > > IPCUnixSocket::recvData() as the number of fds is not bound-checked\n> > > locally, but we will run out of file descriptors before we could\n> > > overflow the buffer size calculation.\n> >\n> > The maximum number of file descriptors that can be transferred in a single `sendmsg()`\n> > call has been 253 for a long time on Linux.\n> \n> Indeed, another reason why we should be safe.\n> \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > Changes 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> > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > > index 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> &planes)\n> > > */\n> > > void EncoderLibJpeg::compressNV(const std::vector> &planes)\n> > > {\n> > > -\tuint8_t tmprowbuf[compress_.image_width * 3];\n> > > +\tstd::vector 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> &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;\n> > > diff --git a/src/apps/common/dng_writer.cpp b/src/apps/common/dng_writer.cpp\n> > > index 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 \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \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 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(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(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);\n> > > diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> > > index ab19aa3d48e7..ece268d0e344 100644\n> > > --- a/src/apps/common/options.cpp\n> > > +++ b/src/apps/common/options.cpp\n> > > @@ -10,6 +10,7 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \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 shortOptions(optionsMap_.size() * 3 + 2);\n> > > +\tstd::vector 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;\n> > > diff --git a/src/ipa/rpi/controller/rpi/alsc.cpp b/src/ipa/rpi/controller/rpi/alsc.cpp\n> > > index 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 \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \n> > > #include \n> > > @@ -496,8 +497,9 @@ void resampleCalTable(const Array2D &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 xLo(X);\n> > > +\tstd::vector xHi(X);\n> > > +\tstd::vector xf(X);\n> > > \tdouble scaleX = cameraMode.sensorWidth /\n> > > \t\t\t(cameraMode.width * cameraMode.scaleX);\n> > > \tdouble xOff = cameraMode.cropX / (double)cameraMode.sensorWidth;\n> > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > > index 75285b679eac..002053e35557 100644\n> > > --- a/src/libcamera/ipc_unixsocket.cpp\n> > > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > > @@ -12,6 +12,7 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \n> > > #include \n> > > @@ -247,10 +248,9 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> > > \tiov[0].iov_base = const_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > >\n> > > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > > +\tstruct cmsghdr *cmsg = reinterpret_cast(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 buf(CMSG_SPACE(num * sizeof(uint32_t)));\n> > >\n> > > -\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > > +\tstruct cmsghdr *cmsg = reinterpret_cast(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> > > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > > index 2546882da085..f39bd986b8ae 100644\n> > > --- a/test/ipc/unixsocket.cpp\n> > > +++ b/test/ipc/unixsocket.cpp\n> > > @@ -15,6 +15,7 @@\n> > > #include \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \n> > > #include \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 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> > >\n> > > base-commit: c9152bad5ce905f5a31dbd05b40195f02c0cc2a9\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BEB4BBDC71\n\tfor ;\n\tTue, 30 Jul 2024 22:39:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9424F63374;\n\tWed, 31 Jul 2024 00:39:10 +0200 (CEST)","from mail-4316.protonmail.ch (mail-4316.protonmail.ch\n\t[185.70.43.16])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61EBB61984\n\tfor ;\n\tWed, 31 Jul 2024 00:39:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"BgT9pNBb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1722379147; x=1722638347;\n\tbh=gcBFhvrQSmZXczvkG+v1gCo4fCgELMoQH3lq6BAoY5E=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=BgT9pNBbDxan1J507sCgqVhEKnc7DNoV42GXW12tzwc7p5VfVDwOZkIYPGhNWK8IS\n\tXv5N4m8eCwmQEdFGqYTLUO2fHjxdr9XccRVAze3mmkfZwkhdynPEBlfP+LG4+L+V0S\n\tyl+8JizJmFiCcH5Ev8pO2SnLz6tf8yHOOCKXW8GexxLlN67HUJERvxRE5oiwHB+kQF\n\ttrw3LAwnEn2LgWwfWz/IQifyzXt1jXmxCDmdK3iMse/S8PazPtmPj7tCHOMjJWTg0N\n\t0vG7Wn/yHjli1RiUI+oXi1OM8KpQGi4gvcz1I61pvMQheC6VjLbI1VLmlk2SBiIr2I\n\tfbaE3lnQLLSew==","Date":"Tue, 30 Jul 2024 22:39:04 +0000","To":"Laurent Pinchart ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] libcamera: Avoid variable-length arrays","Message-ID":"","In-Reply-To":"<20240730222546.GI8146@pendragon.ideasonboard.com>","References":"<20240729182444.16509-1-laurent.pinchart@ideasonboard.com>\n\t\n\t<20240730222546.GI8146@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"356fbb6d25ab9a6d1c9455fb84c57764ffc08610","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]