[{"id":28629,"web_url":"https://patchwork.libcamera.org/comment/28629/","msgid":"<20240201081724.GB11551@pendragon.ideasonboard.com>","date":"2024-02-01T08:17:24","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Khem,\n\nThank you for the patch.\n\nOn Wed, Jan 31, 2024 at 09:08:10PM -0800, Khem Raj wrote:\n> Clang++ 18 is fussy about this with new warning checks.\n> \n>    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n>       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n>           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> \n> Therefore replace using VLAs with alloca and malloc/free\n> \n> Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> ---\n>  src/apps/common/options.cpp      |  4 ++--\n>  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n>  2 files changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index 4f7e8691..b020f603 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -879,8 +879,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> +\tchar *shortOptions = (char*)alloca(optionsMap_.size() * 3 + 2);\n\nWe use C++ casts for C++ code.\n\n> +\tstruct option *longOptions = (struct option*)alloca(optionsMap_.size() + 1);\n>  \tunsigned int ids = 0;\n>  \tunsigned int idl = 0;\n>  \n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 1980d374..3a7f8ee6 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -247,8 +247,8 @@ 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> +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n\nWhy do you allocate on the heap here while you allocate on the stack\nabove ?\n\n> +\tmemset((void*)buf, 0, sizeof(buf));\n>  \n>  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n>  \t\tint ret = -errno;\n>  \t\tLOG(IPCUnixSocket, Error)\n>  \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> +    free(buf);\n\nDid you run checkstyle.py ?\n\nfree() is error-prone. You should use a C++ class that provides RAII\ninstead of freeing memory manually.\n\n>  \t\treturn ret;\n>  \t}\n>  \n> +  free(buf);\n>  \treturn 0;\n>  }\n>  \n> @@ -283,8 +285,8 @@ 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> +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> +\tmemset((void*)buf, 0, sizeof(buf));\n>  \n>  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -305,12 +307,14 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n>  \t\tif (ret != -EAGAIN)\n>  \t\t\tLOG(IPCUnixSocket, Error)\n>  \t\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> +    free(buf);\n>  \t\treturn ret;\n>  \t}\n>  \n>  \tif (fds)\n>  \t\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n>  \n> +  free(buf);\n>  \treturn 0;\n>  }\n>","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 9F66EBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Feb 2024 08:17:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0148A61D15;\n\tThu,  1 Feb 2024 09:17:26 +0100 (CET)","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 763F861D15\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Feb 2024 09:17:24 +0100 (CET)","from pendragon.ideasonboard.com (unknown [94.107.229.70])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C4BC4613C;\n\tThu,  1 Feb 2024 09:16:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"V/6KFPZy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1706775364;\n\tbh=9ZChPnj5WKlneo4QbT3DI3TI1xdJ5qS31eHxjdYXtBs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=V/6KFPZyZISYk9lNcn6v58CZXmxg9RQwdTKjcHhui+BqAlgvEqS2BtVI47D+nEzOD\n\txLKfm5ieatPA0MvtuMfaWHyuEix2vUjY5ns+5HDasJ3/uwfKto2WmIWTSEqUx2YhVx\n\t8CYliqsbRPw4jLW1NhpAWb3qmfuChNfwhPpRVCWE=","Date":"Thu, 1 Feb 2024 10:17:24 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Khem Raj <raj.khem@gmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","Message-ID":"<20240201081724.GB11551@pendragon.ideasonboard.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240201050810.3501276-2-raj.khem@gmail.com>","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28630,"web_url":"https://patchwork.libcamera.org/comment/28630/","msgid":"<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>","date":"2024-02-01T21:48:06","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:\n\nClang++ 18 is fussy about this with new warning checks.\n> \n>    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n>       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n>           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> \n> Therefore replace using VLAs with alloca and malloc/free\n> \n> Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> ---\n>  src/apps/common/options.cpp      |  4 ++--\n>  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n>  2 files changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index 4f7e8691..b020f603 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -879,8 +879,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> +\tchar *shortOptions = (char*)alloca(optionsMap_.size() * 3 + 2);\n> +\tstruct option *longOptions = (struct option*)alloca(optionsMap_.size() + 1);\n\nstd::string usually has SSO, so that could work just fine here. But I suppose\none really wants something like llvm::SmallVector.\n\n\n>  \tunsigned int ids = 0;\n>  \tunsigned int idl = 0;\n> \n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 1980d374..3a7f8ee6 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -247,8 +247,8 @@ 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> +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> +\tmemset((void*)buf, 0, sizeof(buf));\n\nThe linux kernel has had a limit of 253 file descriptors for the last 13 years,\nso I think a fixed size array is fine here.\n\n\n> \n>  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n>  \t\tint ret = -errno;\n>  \t\tLOG(IPCUnixSocket, Error)\n>  \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> +    free(buf);\n>  \t\treturn ret;\n>  \t}\n> \n> +  free(buf);\n>  \treturn 0;\n>  }\n> \n> @@ -283,8 +285,8 @@ 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> +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> +\tmemset((void*)buf, 0, sizeof(buf));\n\nSame here.\n\n\n> \n>  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -305,12 +307,14 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n>  \t\tif (ret != -EAGAIN)\n>  \t\t\tLOG(IPCUnixSocket, Error)\n>  \t\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> +    free(buf);\n>  \t\treturn ret;\n>  \t}\n> \n>  \tif (fds)\n>  \t\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> \n> +  free(buf);\n>  \treturn 0;\n>  }\n> \n> --\n> 2.43.0\n> \n\n\nRegards,\nBarnabás Pőcze","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 825EEBDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Feb 2024 21:48:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5F6462805;\n\tThu,  1 Feb 2024 22:48:22 +0100 (CET)","from mail-40133.protonmail.ch (mail-40133.protonmail.ch\n\t[185.70.40.133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DBD561D1A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Feb 2024 22:48:21 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"vcj9yLF3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1706824100; x=1707083300;\n\tbh=beAr2fg+t84hz/OauoYfZnzbRZqg8TeQcJi3MMy38Gg=;\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=vcj9yLF3oWXoZKKLuYS+/DQaimt1w6lEPKgmrAlMYYkEhGfSZchLUywMopJtLL3fx\n\t+ob2CJoXUPhbJmWAyTDzFE5ExoTDJHFRpvrtA1zT5WUrHykZj2gRRfxBLdltGTiGmm\n\tivwJ3foD/Wh34MDfvGMhINVOeT0oGjFmyN3c+VeaY0WGZh/GPANX+waVQHRHHtUOxB\n\tuLZhxP3qsfAfPqs0NVXVrqtOgq02shbSQ6PLNy2Zqzzudn8PkcYiPWw5lgwPPYonXE\n\tOige6URku9uJl/9igWPusj9wIC4hWwSDu+AHDVAqdsg2oDhCyDwxNhbyplA8nOxF5+\n\t9FVbx5+xyXLYA==","Date":"Thu, 01 Feb 2024 21:48:06 +0000","To":"Khem Raj <raj.khem@gmail.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","Message-ID":"<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>","In-Reply-To":"<20240201050810.3501276-2-raj.khem@gmail.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>","Feedback-ID":"20568564:user:proton","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":"<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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28634,"web_url":"https://patchwork.libcamera.org/comment/28634/","msgid":"<20240204101948.GM3094@pendragon.ideasonboard.com>","date":"2024-02-04T10:19:48","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:\n> 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:\n> \n> Clang++ 18 is fussy about this with new warning checks.\n> > \n> >    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n> >       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n> >           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> > \n> > Therefore replace using VLAs with alloca and malloc/free\n> > \n> > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > ---\n> >  src/apps/common/options.cpp      |  4 ++--\n> >  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n> >  2 files changed, 10 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> > index 4f7e8691..b020f603 100644\n> > --- a/src/apps/common/options.cpp\n> > +++ b/src/apps/common/options.cpp\n> > @@ -879,8 +879,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> > +\tchar *shortOptions = (char*)alloca(optionsMap_.size() * 3 + 2);\n> > +\tstruct option *longOptions = (struct option*)alloca(optionsMap_.size() + 1);\n> \n> std::string usually has SSO, so that could work just fine here. But I suppose\n> one really wants something like llvm::SmallVector.\n\nThat could be nice. Given that we're really not in a hot path, a\nstd::string (possibly with reserve()) is likely the best option here for\nshortOptions, and std::vector should be fine for longOptions.\n\n> >  \tunsigned int ids = 0;\n> >  \tunsigned int idl = 0;\n> > \n> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > index 1980d374..3a7f8ee6 100644\n> > --- a/src/libcamera/ipc_unixsocket.cpp\n> > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > @@ -247,8 +247,8 @@ 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> > +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> > +\tmemset((void*)buf, 0, sizeof(buf));\n> \n> The linux kernel has had a limit of 253 file descriptors for the last 13 years,\n\nDoes it ?\n\n$ ulimit -Hn\n4096\n$ ulimit -Sn\n1024\n\nWe should never have a large number of file descriptors here though, so\na fixed limit should be fine (I think 16 should be more than enough),\nwith proper error checking and handling, as well as a mention in the\ndocumentation.\n\nPaul, what do you think ?\n\n> so I think a fixed size array is fine here.\n> \n> >  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> >  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> >  \t\tint ret = -errno;\n> >  \t\tLOG(IPCUnixSocket, Error)\n> >  \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> > +    free(buf);\n> >  \t\treturn ret;\n> >  \t}\n> > \n> > +  free(buf);\n> >  \treturn 0;\n> >  }\n> > \n> > @@ -283,8 +285,8 @@ 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> > +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> > +\tmemset((void*)buf, 0, sizeof(buf));\n> \n> Same here.\n> \n> > \n> >  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> >  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > @@ -305,12 +307,14 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n> >  \t\tif (ret != -EAGAIN)\n> >  \t\t\tLOG(IPCUnixSocket, Error)\n> >  \t\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> > +    free(buf);\n> >  \t\treturn ret;\n> >  \t}\n> > \n> >  \tif (fds)\n> >  \t\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> > \n> > +  free(buf);\n> >  \treturn 0;\n> >  }\n> >","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 34FB9BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  4 Feb 2024 10:19:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8095E62805;\n\tSun,  4 Feb 2024 11:19:50 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35ABF61CBA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  4 Feb 2024 11:19:48 +0100 (CET)","from pendragon.ideasonboard.com (unknown [109.128.141.99])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3EDBC720;\n\tSun,  4 Feb 2024 11:18:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XescNmP8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1707041906;\n\tbh=6q6PXgdqfauCf/jEkcvL1PVFJJ+LEPHwwlpyfkcRSCk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XescNmP83GzdQ5yw2llcvsFssmMT4Os3ynQKKRZu6lqGtYEr5fgEIK+H9Bon4RtOP\n\tzad5ZmAo4o2MxOjFWRcobjT6jeb9mqROT84E5d3nUHaakgFX2HSbugxwUyHK3+76ui\n\tUjxPO3QMITwTgbzmal39qI9nRf9Gj0ShNDbawoPM=","Date":"Sun, 4 Feb 2024 12:19:48 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","Message-ID":"<20240204101948.GM3094@pendragon.ideasonboard.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>\n\t<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>","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>","Cc":"libcamera-devel@lists.libcamera.org, Khem Raj <raj.khem@gmail.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28635,"web_url":"https://patchwork.libcamera.org/comment/28635/","msgid":"<5dMWrf8VqUGgQJnyI_gKRXXzeG1neOSRmWM4Ru97M1MeDptUsJFywMuGGmPlB1b4_fAiNbSZSBqxAR-e_ZKJQOQyaGBsEtu4qWdiRIdM78s=@protonmail.com>","date":"2024-02-04T14:39:24","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2024. február 4., vasárnap 11:19 keltezéssel, Laurent Pinchart írta:\n\nOn Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:\n> > 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:\n> >\n> > Clang++ 18 is fussy about this with new warning checks.\n> > >\n> > >    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n> > >       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n> > >           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> > >\n> > > Therefore replace using VLAs with alloca and malloc/free\n> > >\n> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > > ---\n> > >  src/apps/common/options.cpp      |  4 ++--\n> > >  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n> > >  2 files changed, 10 insertions(+), 6 deletions(-)\n> > >\n> [...]\n> > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > > index 1980d374..3a7f8ee6 100644\n> > > --- a/src/libcamera/ipc_unixsocket.cpp\n> > > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > > @@ -247,8 +247,8 @@ 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> > > +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> > > +\tmemset((void*)buf, 0, sizeof(buf));\n> >\n> > The linux kernel has had a limit of 253 file descriptors for the last 13 years,\n> \n> Does it ?\n> \n> $ ulimit -Hn\n> 4096\n> $ ulimit -Sn\n> 1024\n> \n> We should never have a large number of file descriptors here though, so\n> a fixed limit should be fine (I think 16 should be more than enough),\n> with proper error checking and handling, as well as a mention in the\n> documentation.\n> \n> Paul, what do you think ?\n> \n\nOops, I meant for SCM_RIGHTS:\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/net/scm.h?id=4e94ddfe2aab72139acb8d5372fac9e6c3f3e383#n18\n(see SCM_MAX_FD)\n\n\n> > so I think a fixed size array is fine here.\n> >\n> > >  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > >  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > > @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> > >  \t\tint ret = -errno;\n> > >  \t\tLOG(IPCUnixSocket, Error)\n> > >  \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> > > +    free(buf);\n> > >  \t\treturn ret;\n> > >  \t}\n> > >\n> > > +  free(buf);\n> > >  \treturn 0;\n> > >  }\n> [...]\n\nRegards,\nBarnabás Pőcze","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 893AEBDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  4 Feb 2024 14:39:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EA92261D11;\n\tSun,  4 Feb 2024 15:39:37 +0100 (CET)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC92561CBE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  4 Feb 2024 15:39:35 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"IplDA3A2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1707057575; x=1707316775;\n\tbh=2Kyyh1WAzVCAC89ww2qYOzel+4QlcNZ5hNSjuvNfn5g=;\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=IplDA3A2FrCWT8b0aLRDqRtjMGqk0PaHgJ1EAfjkQ6g2ysKEDhSe5NImPa7bEJhsn\n\tFALxuMq26Sbkrje0OAYatj8Zcf1XjYPZo6JNsJtMwfXtHijyhZoZZxaqLO3j+y+XuA\n\tDZvCy1hY9jVkXmTw4tTU1sl28R7a+UeJnHYcTRpLFa3oPJUNU2n5n9pIdUYqz6+7Iv\n\tgW729spHouoQ+QRud0VRSCkn/SGd3zgyo8qH8sgSMbxuJ0sCepXAomaILWv8z/cQEH\n\tO44lBBg82XzoxgzBdDYwI93O6DuS77LIAzDLK+uaPhoQ3wJte60kHJ93LiFrGRLnJi\n\t9X2odxvp2Eobw==","Date":"Sun, 04 Feb 2024 14:39:24 +0000","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","Message-ID":"<5dMWrf8VqUGgQJnyI_gKRXXzeG1neOSRmWM4Ru97M1MeDptUsJFywMuGGmPlB1b4_fAiNbSZSBqxAR-e_ZKJQOQyaGBsEtu4qWdiRIdM78s=@protonmail.com>","In-Reply-To":"<20240204101948.GM3094@pendragon.ideasonboard.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>\n\t<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>\n\t<20240204101948.GM3094@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","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":"<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>","Cc":"libcamera-devel@lists.libcamera.org, Khem Raj <raj.khem@gmail.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28636,"web_url":"https://patchwork.libcamera.org/comment/28636/","msgid":"<20240204145742.GC6804@pendragon.ideasonboard.com>","date":"2024-02-04T14:57:42","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Feb 04, 2024 at 02:39:24PM +0000, Barnabás Pőcze wrote:\n> 2024. február 4., vasárnap 11:19 keltezéssel, Laurent Pinchart írta:\n> On Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:\n> > > 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:\n> > >\n> > > Clang++ 18 is fussy about this with new warning checks.\n> > > >\n> > > >    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n> > > >       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n> > > >           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> > > >\n> > > > Therefore replace using VLAs with alloca and malloc/free\n> > > >\n> > > > Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> > > > ---\n> > > >  src/apps/common/options.cpp      |  4 ++--\n> > > >  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n> > > >  2 files changed, 10 insertions(+), 6 deletions(-)\n> > > >\n> > [...]\n> > > > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > > > index 1980d374..3a7f8ee6 100644\n> > > > --- a/src/libcamera/ipc_unixsocket.cpp\n> > > > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > > > @@ -247,8 +247,8 @@ 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> > > > +\tchar *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> > > > +\tmemset((void*)buf, 0, sizeof(buf));\n> > >\n> > > The linux kernel has had a limit of 253 file descriptors for the last 13 years,\n> > \n> > Does it ?\n> > \n> > $ ulimit -Hn\n> > 4096\n> > $ ulimit -Sn\n> > 1024\n> > \n> > We should never have a large number of file descriptors here though, so\n> > a fixed limit should be fine (I think 16 should be more than enough),\n> > with proper error checking and handling, as well as a mention in the\n> > documentation.\n> > \n> > Paul, what do you think ?\n> \n> Oops, I meant for SCM_RIGHTS:\n> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/net/scm.h?id=4e94ddfe2aab72139acb8d5372fac9e6c3f3e383#n18\n> (see SCM_MAX_FD)\n\nAh, I didn't know that. Thanks for pointing it out.\n\n> > > so I think a fixed size array is fine here.\n> > >\n> > > >  \tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > > >  \tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > > > @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n> > > >  \t\tint ret = -errno;\n> > > >  \t\tLOG(IPCUnixSocket, Error)\n> > > >  \t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> > > > +    free(buf);\n> > > >  \t\treturn ret;\n> > > >  \t}\n> > > >\n> > > > +  free(buf);\n> > > >  \treturn 0;\n> > > >  }\n> >\n> > [...]","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 45293BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  4 Feb 2024 14:57:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A743661CBE;\n\tSun,  4 Feb 2024 15:57:43 +0100 (CET)","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 EC6C161CBE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  4 Feb 2024 15:57:41 +0100 (CET)","from pendragon.ideasonboard.com (unknown [151.216.142.210])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CC8CB6F2;\n\tSun,  4 Feb 2024 15:56:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tBStfiBm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1707058580;\n\tbh=iTgyGH1R/hVw0tetQQDpPmtrjw8S4yQqLiJOErUcKdI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tBStfiBm7m9gOfCvgpHmbjrfpUJWUdy+BLqkQ1l6RUgCc8Vs0Y5r4Mf1FIcFRiOVz\n\trPZrmbg+BrdxXRhZVnVwf3sItExpKd0UPup9Qo0SE0jrpd9ihb+WljYcITRj4zyxzh\n\tkMYn4KWetPh6rxFOrCWLp76HH1DsIzzg3+Xqy8L8=","Date":"Sun, 4 Feb 2024 16:57:42 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","Message-ID":"<20240204145742.GC6804@pendragon.ideasonboard.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>\n\t<mXFx9WOr55wWR0pfOrtGETDb6oyYV7SHW2Y8hsegLFeNvLCDboPZt24uozFkR_82uSM5ubAFsrQ_GWTD1tc-sV4eKEaQ7qSa-y-E4aqcb1c=@protonmail.com>\n\t<20240204101948.GM3094@pendragon.ideasonboard.com>\n\t<5dMWrf8VqUGgQJnyI_gKRXXzeG1neOSRmWM4Ru97M1MeDptUsJFywMuGGmPlB1b4_fAiNbSZSBqxAR-e_ZKJQOQyaGBsEtu4qWdiRIdM78s=@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5dMWrf8VqUGgQJnyI_gKRXXzeG1neOSRmWM4Ru97M1MeDptUsJFywMuGGmPlB1b4_fAiNbSZSBqxAR-e_ZKJQOQyaGBsEtu4qWdiRIdM78s=@protonmail.com>","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>","Cc":"libcamera-devel@lists.libcamera.org, Khem Raj <raj.khem@gmail.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29016,"web_url":"https://patchwork.libcamera.org/comment/29016/","msgid":"<171095327790.252503.3856539603999738407@ping.linuxembedded.co.uk>","date":"2024-03-20T16:47:57","subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Khem,\n\nQuoting Khem Raj (2024-02-01 05:08:10)\n> Clang++ 18 is fussy about this with new warning checks.\n> \n>    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]\n>       882 |         char shortOptions[optionsMap_.size() * 3 + 2];\n>           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~\n> \n> Therefore replace using VLAs with alloca and malloc/free\n\nI'm not sure which patch specifically causes the fault - But I would\nguess this one to start with.\n\nhttps://bugs.libcamera.org/show_bug.cgi?id=214 \"[RISC-V] stack smashing\ndetected\" reports that building libcamera with yocto causes runtime\nfaults in cam, and the user reports that your three patches are applied\non top.\n\nI've asked that they report that directly to you in the yocto project,\nas that's where the fault is currently, but I figure it makes sense to\nreport the potential issue associated with this series here too.\n\nPlease take a look.\n\nRegards\n\nKieran\n\n\n> Signed-off-by: Khem Raj <raj.khem@gmail.com>\n> ---\n>  src/apps/common/options.cpp      |  4 ++--\n>  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----\n>  2 files changed, 10 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp\n> index 4f7e8691..b020f603 100644\n> --- a/src/apps/common/options.cpp\n> +++ b/src/apps/common/options.cpp\n> @@ -879,8 +879,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)\n>          * Allocate short and long options arrays large enough to contain all\n>          * options.\n>          */\n> -       char shortOptions[optionsMap_.size() * 3 + 2];\n> -       struct option longOptions[optionsMap_.size() + 1];\n> +       char *shortOptions = (char*)alloca(optionsMap_.size() * 3 + 2);\n> +       struct option *longOptions = (struct option*)alloca(optionsMap_.size() + 1);\n>         unsigned int ids = 0;\n>         unsigned int idl = 0;\n>  \n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> index 1980d374..3a7f8ee6 100644\n> --- a/src/libcamera/ipc_unixsocket.cpp\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -247,8 +247,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n>         iov[0].iov_base = const_cast<void *>(buffer);\n>         iov[0].iov_len = length;\n>  \n> -       char buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> -       memset(buf, 0, sizeof(buf));\n> +       char *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> +       memset((void*)buf, 0, sizeof(buf));\n>  \n>         struct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>         cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,\n>                 int ret = -errno;\n>                 LOG(IPCUnixSocket, Error)\n>                         << \"Failed to sendmsg: \" << strerror(-ret);\n> +    free(buf);\n>                 return ret;\n>         }\n>  \n> +  free(buf);\n>         return 0;\n>  }\n>  \n> @@ -283,8 +285,8 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n>         iov[0].iov_base = buffer;\n>         iov[0].iov_len = length;\n>  \n> -       char buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> -       memset(buf, 0, sizeof(buf));\n> +       char *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));\n> +       memset((void*)buf, 0, sizeof(buf));\n>  \n>         struct cmsghdr *cmsg = (struct cmsghdr *)buf;\n>         cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> @@ -305,12 +307,14 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,\n>                 if (ret != -EAGAIN)\n>                         LOG(IPCUnixSocket, Error)\n>                                 << \"Failed to recvmsg: \" << strerror(-ret);\n> +    free(buf);\n>                 return ret;\n>         }\n>  \n>         if (fds)\n>                 memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n>  \n> +  free(buf);\n>         return 0;\n>  }\n>  \n> -- \n> 2.43.0\n>","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 19A8BBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 20 Mar 2024 16:48:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C5D463055;\n\tWed, 20 Mar 2024 17:48:03 +0100 (CET)","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 430E1603AC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Mar 2024 17:48:01 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 94AC2B1;\n\tWed, 20 Mar 2024 17:47:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pPlkReqh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710953253;\n\tbh=xTqT8v9TcN9W3xDJWiz2aCwNPok2mCcB/1Gi0UrGFpQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pPlkReqht6cD/V7ueD2i0NQCF4NYHhQhXsOVqyyDHPA8zbPe2IwML+KqCNLVKRC6L\n\tBLHpiFAT3lkcy01G+vISLZDBhI9jxFKTQDigwvCsQwTJZRQaNtGUG8cTC55orTqXPL\n\tJJapm0/cWIcWk/eH+Pl+GAR7dlsh1qKD2lomT4po=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240201050810.3501276-2-raj.khem@gmail.com>","References":"<20240201050810.3501276-1-raj.khem@gmail.com>\n\t<20240201050810.3501276-2-raj.khem@gmail.com>","Subject":"Re: [PATCH 2/2] options: Replace use of VLAs in C++","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Khem Raj <raj.khem@gmail.com>","To":"Khem Raj <raj.khem@gmail.com>, libcamera-devel@lists.libcamera.org","Date":"Wed, 20 Mar 2024 16:47:57 +0000","Message-ID":"<171095327790.252503.3856539603999738407@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<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>"}}]