Re: [PATCH 3/3] tools/nolibc: simplify mode handling in open() and openat()

[Willy Tarreau <w@1wt.eu>]

On Sun, Apr 19, 2026 at 05:29:05PM +0200, Thomas Weißschuh wrote:
> The current handling of the optional mode arguments using va_list has
> some drawbacks. It is hard for the compiler to optimize away and it
> needs specific code to handle the O_ flags that need to pass the mode
> parameter. Currently that mode parameter is not respected for O_TMPFILE,
> which is a bug.
> 
> Switch to a macro-based variant which does not generate any additional
> code and avoid the explicit handling of 'mode'.
> The macros require somewhat recent compiler versions, but users stuck on
> old compilers have a trivial workaround by always specifying mode.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> 
> ---
> It might also possible to use macros similar to syscall() to not require
> __VA_OPT__, but those would be fairly ugly.
> ---
>  tools/include/nolibc/compiler.h |  2 ++
>  tools/include/nolibc/fcntl.h    | 34 +++++++++++++---------------------
>  2 files changed, 15 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/include/nolibc/compiler.h b/tools/include/nolibc/compiler.h
> index b56570bf9f69..a154aa4e143a 100644
> --- a/tools/include/nolibc/compiler.h
> +++ b/tools/include/nolibc/compiler.h
> @@ -90,4 +90,6 @@
>  #  define __nolibc_no_sanitize_undefined
>  #endif
>  
> +#define __nolibc_first_arg(_a1, ...) _a1
> +
>  #endif /* _NOLIBC_COMPILER_H */
> diff --git a/tools/include/nolibc/fcntl.h b/tools/include/nolibc/fcntl.h
> index 56650a36f856..89436a72e987 100644
> --- a/tools/include/nolibc/fcntl.h
> +++ b/tools/include/nolibc/fcntl.h
> @@ -25,18 +25,8 @@ int _sys_openat(int dirfd, const char *path, int flags, mode_t mode)
>  }
>  
>  static __attribute__((unused))
> -int openat(int dirfd, const char *path, int flags, ...)
> +int openat(int dirfd, const char *path, int flags, mode_t mode)
>  {
> -	mode_t mode = 0;
> -
> -	if (flags & O_CREAT) {
> -		va_list args;
> -
> -		va_start(args, flags);
> -		mode = va_arg(args, mode_t);
> -		va_end(args);
> -	}
> -
>  	return __sysret(_sys_openat(dirfd, path, flags, mode));
>  }
>  
> @@ -51,20 +41,22 @@ int _sys_open(const char *path, int flags, mode_t mode)
>  }
>  
>  static __attribute__((unused))
> -int open(const char *path, int flags, ...)
> +int open(const char *path, int flags, mode_t mode)
>  {
> -	mode_t mode = 0;
> +	return __sysret(_sys_open(path, flags, mode));
> +}
>  
> -	if (flags & O_CREAT) {
> -		va_list args;
> +#if __nolibc_gnuc_version >= __nolibc_version(8, 0, 0) || \
> +	__nolibc_clang_version >= __nolibc_version(12, 0, 0)
>  
> -		va_start(args, flags);
> -		mode = va_arg(args, mode_t);
> -		va_end(args);
> -	}
> +#  define __nolibc_open_mode(...) __nolibc_first_arg(__VA_ARGS__ __VA_OPT__(,) 0)
>  
> -	return __sysret(_sys_open(path, flags, mode));
> -}
> +#  define open(path, flags, ...) \
> +	  open(path, flags, __nolibc_open_mode(__VA_ARGS__))
> +#  define openat(dirfd, path, flags, ...) \
> +	  openat(dirfd, path, flags, __nolibc_open_mode(__VA_ARGS__))
> +
> +#endif
>  
>  /*
>   * int creat(const char *path, mode_t mode);

I'm really confused here. It looks to me like we first define the open()
function then define it as a macro that will override it for recent
compilers. It doesn't make the code easy to follow, and in addition I'm
really seeing this as a step backwards, for having had to systematically
modify existing code to append ',0' to all open() calls in order to build
with nolibc years ago (hence the leftover that you spotted in the test).

It's not clear to me what was the problem you faced with O_TMPFILE in the
first place. If it's just a matter of making the 3rd arg optional and not
depend on a valist, we can instead define open() as a macro that always
passes the 3rd arg as zero when not defined. When I need to handle varargs
in macros, I do it like this:

  #define _OPT_ARG(a0, a1, ...) a1
  #define open(path, flags, mode...) _open(path, flags, _OPT_ARG(0, ##mode, 0))

  int _open(const char *path, int flags, mode_t mode)
  {
     ...
  }

The macro passes the "mode" argument to _open() when it's present, and when
it's absent, it passes the next one in the _OPT_ARG() macro, which is zero.

E.g.:

  #include <stdio.h>
  
  #define _OPT_ARG(a0, a1, ...) a1
  #define open(path, flags, mode...) _open(path, flags, _OPT_ARG(0, ##mode, 0))
  
  int _open(const char *path, int flags, int mode)
  {
     return printf("path=%s flags=%d mode=%d\n", path, flags, mode);
  }
  
  int main(void)
  {
          open("file1", 12);
          open("file2", 34, 56);
          return 0;
  }

  $ ./a.out
  path=file1 flags=12 mode=0
  path=file2 flags=34 mode=56

And this one works even on very old compilers (gcc-3.4 successfully tested).

Willy