[2/2] options: Replace use of VLAs in C++
diff mbox series

Message ID 20240201050810.3501276-2-raj.khem@gmail.com
State New
Headers show
Series
  • [1/2] media_device: Add bool return type to unlock()
Related show

Commit Message

Khem Raj Feb. 1, 2024, 5:08 a.m. UTC
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

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 src/apps/common/options.cpp      |  4 ++--
 src/libcamera/ipc_unixsocket.cpp | 12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

Comments

Laurent Pinchart Feb. 1, 2024, 8:17 a.m. UTC | #1
Hi Khem,

Thank you for the patch.

On Wed, Jan 31, 2024 at 09:08:10PM -0800, Khem Raj wrote:
> 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
> 
> Signed-off-by: Khem Raj <raj.khem@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);

We use C++ casts for C++ code.

> +	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)));

Why do you allocate on the heap here while you allocate on the stack
above ?

> +	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);

Did you run checkstyle.py ?

free() is error-prone. You should use a C++ class that provides RAII
instead of freeing memory manually.

>  		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;
>  }
>
Barnabás Pőcze Feb. 1, 2024, 9:48 p.m. UTC | #2
Hi


2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:

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
> 
> Signed-off-by: Khem Raj <raj.khem@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);

std::string usually has SSO, so that could work just fine here. But I suppose
one really wants something like llvm::SmallVector.


>  	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));

The linux kernel has had a limit of 253 file descriptors for the last 13 years,
so I think a fixed size array is fine here.


> 
>  	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));

Same here.


> 
>  	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
> 


Regards,
Barnabás Pőcze
Laurent Pinchart Feb. 4, 2024, 10:19 a.m. UTC | #3
On Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:
> 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:
> 
> 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
> > 
> > Signed-off-by: Khem Raj <raj.khem@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);
> 
> std::string usually has SSO, so that could work just fine here. But I suppose
> one really wants something like llvm::SmallVector.

That could be nice. Given that we're really not in a hot path, a
std::string (possibly with reserve()) is likely the best option here for
shortOptions, and std::vector should be fine for longOptions.

> >  	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));
> 
> The linux kernel has had a limit of 253 file descriptors for the last 13 years,

Does it ?

$ ulimit -Hn
4096
$ ulimit -Sn
1024

We should never have a large number of file descriptors here though, so
a fixed limit should be fine (I think 16 should be more than enough),
with proper error checking and handling, as well as a mention in the
documentation.

Paul, what do you think ?

> so I think a fixed size array is fine here.
> 
> >  	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));
> 
> Same here.
> 
> > 
> >  	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;
> >  }
> >
Barnabás Pőcze Feb. 4, 2024, 2:39 p.m. UTC | #4
Hi


2024. február 4., vasárnap 11:19 keltezéssel, Laurent Pinchart írta:

On Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:
> > 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:
> >
> > 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
> > >
> > > Signed-off-by: Khem Raj <raj.khem@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/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));
> >
> > The linux kernel has had a limit of 253 file descriptors for the last 13 years,
> 
> Does it ?
> 
> $ ulimit -Hn
> 4096
> $ ulimit -Sn
> 1024
> 
> We should never have a large number of file descriptors here though, so
> a fixed limit should be fine (I think 16 should be more than enough),
> with proper error checking and handling, as well as a mention in the
> documentation.
> 
> Paul, what do you think ?
> 

Oops, I meant for SCM_RIGHTS:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/net/scm.h?id=4e94ddfe2aab72139acb8d5372fac9e6c3f3e383#n18
(see SCM_MAX_FD)


> > so I think a fixed size array is fine here.
> >
> > >  	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;
> > >  }
> [...]

Regards,
Barnabás Pőcze
Laurent Pinchart Feb. 4, 2024, 2:57 p.m. UTC | #5
On Sun, Feb 04, 2024 at 02:39:24PM +0000, Barnabás Pőcze wrote:
> 2024. február 4., vasárnap 11:19 keltezéssel, Laurent Pinchart írta:
> On Thu, Feb 01, 2024 at 09:48:06PM +0000, Barnabás Pőcze wrote:
> > > 2024. február 1., csütörtök 6:08 keltezéssel, Khem Raj írta:
> > >
> > > 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
> > > >
> > > > Signed-off-by: Khem Raj <raj.khem@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/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));
> > >
> > > The linux kernel has had a limit of 253 file descriptors for the last 13 years,
> > 
> > Does it ?
> > 
> > $ ulimit -Hn
> > 4096
> > $ ulimit -Sn
> > 1024
> > 
> > We should never have a large number of file descriptors here though, so
> > a fixed limit should be fine (I think 16 should be more than enough),
> > with proper error checking and handling, as well as a mention in the
> > documentation.
> > 
> > Paul, what do you think ?
> 
> Oops, I meant for SCM_RIGHTS:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/net/scm.h?id=4e94ddfe2aab72139acb8d5372fac9e6c3f3e383#n18
> (see SCM_MAX_FD)

Ah, I didn't know that. Thanks for pointing it out.

> > > so I think a fixed size array is fine here.
> > >
> > > >  	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;
> > > >  }
> >
> > [...]
Kieran Bingham March 20, 2024, 4:47 p.m. UTC | #6
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@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
>

Patch
diff mbox series

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;
 }