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

[Barnabás Pőcze]

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


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