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

[Nicolas Dufresne]

Hi,

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 ?

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