[PATCH v2 2/4] libcamera: base: memfd: Handle uClibc compatibility with function wrapper

[Laurent Pinchart]

Hi Nicolas,

On Wed, Jul 31, 2024 at 02:49:25PM -0400, Nicolas Dufresne wrote:
> Le mercredi 31 juillet 2024 à 16:59 +0300, Laurent Pinchart a écrit :
> > uClibc doesn't provide memfd_create(), which led libcamera to open-code
> > the call using syscall(). Sprinkling the code with #ifdef's isn't the
> > most readable option, so improve it by providing a local implementation
> > of memfd_create(), and call the function unconditionally from
> > MemFd::create(). This makes the main code path more readable.
> > 
> > Suggested-by: Nicolas Dufresne <nicolas at ndufresne.ca>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/base/memfd.cpp | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libcamera/base/memfd.cpp b/src/libcamera/base/memfd.cpp
> > index 72474307af09..d95adb91615c 100644
> > --- a/src/libcamera/base/memfd.cpp
> > +++ b/src/libcamera/base/memfd.cpp
> > @@ -20,15 +20,26 @@
> >   * \brief Anonymous file creation
> >   */
> >  
> > -/* uClibc doesn't provide the file sealing API. */
> >  #ifndef __DOXYGEN__
> > +namespace {
> > +
> > +/* uClibc doesn't provide the file sealing API. */
> >  #if not HAVE_FILE_SEALS
> >  #define F_ADD_SEALS		1033
> >  #define F_SEAL_SHRINK		0x0002
> >  #define F_SEAL_GROW		0x0004
> >  #endif
> > +
> > +#ifndef HAVE_MEMFD_CREATE
> > +int memfd_create(const char *name, unsigned int flags)
> 
> This is not being exported, so perhaps `static int ..` Or is that the default i
> .cpp ?

The preferred way to make symbols local in C++ is to enclose them in an
anonymous namespace:

/* This will be visible outside of the compilation unit */
int foo()
{
	return 42;
}

namespace {

/* This won't */

int bar()
{
	return 24;
}

}

> > +{
> > +	return syscall(SYS_memfd_create, name, flags);
> > +}
> >  #endif
> >  
> > +} /* namespace */
> > +#endif /* __DOXYGEN__ */
> > +
> >  namespace libcamera {
> >  
> >  LOG_DECLARE_CATEGORY(File)
> > @@ -72,11 +83,7 @@ LOG_DECLARE_CATEGORY(File)
> >   */
> >  UniqueFD MemFd::create(const char *name, std::size_t size, Seals seals)
> >  {
> > -#if HAVE_MEMFD_CREATE
> >  	int ret = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
> > -#else
> > -	int ret = syscall(SYS_memfd_create, name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
> > -#endif
> >  	if (ret < 0) {
> >  		ret = errno;
> >  		LOG(File, Error)
> 
> A lot nicer to me (of course I'm biased from having suggested it).
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

-- 
Regards,

Laurent Pinchart