On Fri, Oct 4, 2024 at 1:47 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> Similar to struct fd; unlike struct fd, it can represent
> error values.
>
> Accessors:
>
> * fd_empty(f): true if f represents an error
> * fd_file(f): just as for struct fd it yields a pointer to
> struct file if fd_empty(f) is false. If
> fd_empty(f) is true, fd_file(f) is guaranteed
> _not_ to be an address of any object (IS_ERR()
> will be true in that case)
> * fd_err(f): if f represents an error, returns that error,
> otherwise the return value is junk.
>
> Constructors:
>
> * ERR_FDERR(-E...): an instance encoding given error [ERR_FDERR, perhaps?]
> * BORROWED_FDERR(file): if file points to a struct file instance,
> return a struct fderr representing that file
> reference with no flags set.
> if file is an ERR_PTR(-E...), return a struct
> fderr representing that error.
> file MUST NOT be NULL.
> * CLONED_FDERR(file): similar, but in case when file points to
> a struct file instance, set FDPUT_FPUT in flags.
>
> Same destructor as for struct fd; I'm not entirely convinced that
> playing with _Generic is a good idea here, but for now let's go
> that way...
>
> See fs/overlayfs/file.c for example of use.
I had already posted an alternative code for overlayfs, but in case this
is going to be used anyway in overlayfs or in another code, see some
comments below...
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> fs/overlayfs/file.c | 128 +++++++++++++++++++++----------------------
> include/linux/file.h | 37 +++++++++++--
> 2 files changed, 95 insertions(+), 70 deletions(-)
>
[...]
> diff --git a/include/linux/file.h b/include/linux/file.h
> index f98de143245a..d85352523368 100644
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -44,13 +44,26 @@ static inline void fput_light(struct file *file, int fput_needed)
> struct fd {
> unsigned long word;
> };
> +
> +/* either a reference to struct file + flags
> + * (cloned vs. borrowed, pos locked), with
> + * flags stored in lower bits of value,
> + * or an error (represented by small negative value).
> + */
> +struct fderr {
> + unsigned long word;
> +};
> +
> #define FDPUT_FPUT 1
> #define FDPUT_POS_UNLOCK 2
>
> +#define fd_empty(f) _Generic((f), \
> + struct fd: unlikely(!(f).word), \
> + struct fderr: IS_ERR_VALUE((f).word))
I suggest adding a fd_is_err(f) helper to rhyme with IS_ERR().
> #define fd_file(f) ((struct file *)((f).word & ~(FDPUT_FPUT|FDPUT_POS_UNLOCK)))
> -static inline bool fd_empty(struct fd f)
> +static inline long fd_err(struct fderr f)
> {
> - return unlikely(!f.word);
> + return (long)f.word;
> }
>
> #define EMPTY_FD (struct fd){0}
> @@ -63,11 +76,25 @@ static inline struct fd CLONED_FD(struct file *f)
> return (struct fd){(unsigned long)f | FDPUT_FPUT};
> }
>
> -static inline void fdput(struct fd fd)
> +static inline struct fderr ERR_FDERR(long n)
> +{
> + return (struct fderr){(unsigned long)n};
> +}
> +static inline struct fderr BORROWED_FDERR(struct file *f)
> {
> - if (fd.word & FDPUT_FPUT)
> - fput(fd_file(fd));
> + return (struct fderr){(unsigned long)f};
> }
> +static inline struct fderr CLONED_FDERR(struct file *f)
> +{
> + if (IS_ERR(f))
> + return BORROWED_FDERR(f);
> + return (struct fderr){(unsigned long)f | FDPUT_FPUT};
> +}
> +
> +#define fdput(f) (void) (_Generic((f), \
> + struct fderr: IS_ERR_VALUE((f).word), \
Should that be !IS_ERR_VALUE((f).word)?
> + struct fd: true) && \
> + ((f).word & FDPUT_FPUT) && (fput(fd_file(f)),0))
>
or better yet
#define fd_is_err(f) _Generic((f), \
struct fd: false, \
struct fderr: IS_ERR_VALUE((f).word))
#define fdput(f) (!fd_is_err(f) && ((f).word & FDPUT_FPUT) &&
(fput(fd_file(f)),0))
Thanks,
Amir.
