Re: [PATCH v4 01/47] file: add FD_{ADD,PREPARE}()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Sun, Nov 23, 2025 at 05:33:19PM +0100, Christian Brauner wrote:

> +/*
> + * __FD_PREPARE_INIT(fd_flags, file_init_expr):
> + *     Helper to initialize fd_prepare class.
> + * @fd_flags: flags for get_unused_fd_flags()
> + * @file_init_expr: expression that returns struct file *
> + *
> + * Returns a struct fd_prepare with fd, file, and err set.
> + * If fd allocation fails, fd will be negative and err will be set.
> + * If fd succeeds but file_init_expr fails, file will be ERR_PTR and err will be set.
> + * The err field is the single source of truth for error checking.
> + */
> +#define __FD_PREPARE_INIT(_fd_flags, _file_init_owned)                   \
> +	({                                                               \
> +		class_fd_prepare_t _fd_prepare = {                       \
> +			.__fd = get_unused_fd_flags((_fd_flags)),        \
> +		};                                                       \
> +		if (likely(_fd_prepare.__fd >= 0))                       \
> +			_fd_prepare.__file = (_file_init_owned);         \
> +		_fd_prepare.err = ACQUIRE_ERR(fd_prepare, &_fd_prepare); \
> +		_fd_prepare;                                             \
> +	})
> +
> +/*
> + * FD_PREPARE(var, fd_flags, file_init_owned):
> + *     Declares and initializes an fd_prepare variable with automatic cleanup.
> + *     No separate scope required - cleanup happens when variable goes out of scope.
> + *
> + * @_var: name of struct fd_prepare variable to define
> + * @_fd_flags: flags for get_unused_fd_flags()
> + * @_file_init_owned: struct file to take ownership of (can be expression)
> + */
> +#define FD_PREPARE(_var, _fd_flags, _file_init_owned) \
> +	CLASS_INIT(fd_prepare, _var, __FD_PREPARE_INIT(_fd_flags, _file_init_owned))
> +
> +#define fd_publish(_fd_prepare)                                \
> +	({                                                     \
> +		class_fd_prepare_t *__p = &(_fd_prepare);      \
> +		VFS_WARN_ON_ONCE(__p->err);                    \
> +		VFS_WARN_ON_ONCE(__p->__fd < 0);               \
> +		VFS_WARN_ON_ONCE(IS_ERR_OR_NULL(__p->__file)); \
> +		fd_install(__p->__fd, __p->__file);            \
> +		retain_and_null_ptr(__p->__file);              \
> +		take_fd(__p->__fd);                            \
> +	})

Why not use a real function?

> +#define FD_ADD(_fd_flags, _file_init_owned)                    \
> +	({                                                     \
> +		FD_PREPARE(_var, _fd_flags, _file_init_owned); \
> +		s32 ret = _var.err;                            \
> +		if (likely(!ret))                              \
> +			ret = fd_publish(_var);                \
> +		ret;                                           \
> +	})

Here too, seems like there are many of these, for the sake of .text
why not have it just be a function call?

 int __do_fd_add(int flags, struct file *new_file);
 #define FD_ADD(_fd_flags, new_file) __do_fd_add(_fd_flags, new_file)

cleanup.h is not adding anything to the caller since the cleanup is
fully contained to the scope of the macro.

I don't think it really matters that get_unused_fd_flags() is called
before or after allocating the struct file?

Jason