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

[Laurent Pinchart]

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 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/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;
> > > >  }
> >
> > [...]

-- 
Regards,

Laurent Pinchart