[PATCH 2/2] options: Replace use of VLAs in C++

[Kieran Bingham]

Hi Khem,

Quoting Khem Raj (2024-02-01 05:08:10)
> Clang++ 18 is fussy about this with new warning checks.
> 
>    ../git/src/apps/common/options.cpp:882:20: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
>       882 |         char shortOptions[optionsMap_.size() * 3 + 2];
>           |                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Therefore replace using VLAs with alloca and malloc/free

I'm not sure which patch specifically causes the fault - But I would
guess this one to start with.

https://bugs.libcamera.org/show_bug.cgi?id=214 "[RISC-V] stack smashing
detected" reports that building libcamera with yocto causes runtime
faults in cam, and the user reports that your three patches are applied
on top.

I've asked that they report that directly to you in the yocto project,
as that's where the fault is currently, but I figure it makes sense to
report the potential issue associated with this series here too.

Please take a look.

Regards

Kieran


> Signed-off-by: Khem Raj <raj.khem at gmail.com>
> ---
>  src/apps/common/options.cpp      |  4 ++--
>  src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp
> index 4f7e8691..b020f603 100644
> --- a/src/apps/common/options.cpp
> +++ b/src/apps/common/options.cpp
> @@ -879,8 +879,8 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
>          * Allocate short and long options arrays large enough to contain all
>          * options.
>          */
> -       char shortOptions[optionsMap_.size() * 3 + 2];
> -       struct option longOptions[optionsMap_.size() + 1];
> +       char *shortOptions = (char*)alloca(optionsMap_.size() * 3 + 2);
> +       struct option *longOptions = (struct option*)alloca(optionsMap_.size() + 1);
>         unsigned int ids = 0;
>         unsigned int idl = 0;
>  
> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp
> index 1980d374..3a7f8ee6 100644
> --- a/src/libcamera/ipc_unixsocket.cpp
> +++ b/src/libcamera/ipc_unixsocket.cpp
> @@ -247,8 +247,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
>         iov[0].iov_base = const_cast<void *>(buffer);
>         iov[0].iov_len = length;
>  
> -       char buf[CMSG_SPACE(num * sizeof(uint32_t))];
> -       memset(buf, 0, sizeof(buf));
> +       char *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));
> +       memset((void*)buf, 0, sizeof(buf));
>  
>         struct cmsghdr *cmsg = (struct cmsghdr *)buf;
>         cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));
> @@ -270,9 +270,11 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length,
>                 int ret = -errno;
>                 LOG(IPCUnixSocket, Error)
>                         << "Failed to sendmsg: " << strerror(-ret);
> +    free(buf);
>                 return ret;
>         }
>  
> +  free(buf);
>         return 0;
>  }
>  
> @@ -283,8 +285,8 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
>         iov[0].iov_base = buffer;
>         iov[0].iov_len = length;
>  
> -       char buf[CMSG_SPACE(num * sizeof(uint32_t))];
> -       memset(buf, 0, sizeof(buf));
> +       char *buf = (char*)malloc(CMSG_SPACE(num * sizeof(uint32_t)));
> +       memset((void*)buf, 0, sizeof(buf));
>  
>         struct cmsghdr *cmsg = (struct cmsghdr *)buf;
>         cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));
> @@ -305,12 +307,14 @@ int IPCUnixSocket::recvData(void *buffer, size_t length,
>                 if (ret != -EAGAIN)
>                         LOG(IPCUnixSocket, Error)
>                                 << "Failed to recvmsg: " << strerror(-ret);
> +    free(buf);
>                 return ret;
>         }
>  
>         if (fds)
>                 memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));
>  
> +  free(buf);
>         return 0;
>  }
>  
> -- 
> 2.43.0
>