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

[Hans de Goede]

Hi,

On 7/31/24 3:59 PM, Laurent Pinchart wrote:
> 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

Hmm below you are removing:

"#if HAVE_MEMFD_CREATE"

Which suggests that HAVE_MEMFD_CREATE is always defined
as either 0 or 1 and above you use "#if not HAVE_FILE_SEALS"
I would prefer:

#if not HAVE_MEMFD_CREATE

to match the "#if not HAVE_FILE_SEALS" for consistency.

otherwise this looks good to me:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


> +int memfd_create(const char *name, unsigned int flags)
> +{
> +	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)