Re: [RFC] struct fd situation

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, May 31, 2024 at 11:44:22AM -0700, Linus Torvalds wrote:

> The above obviously assumes that an 'fd_err' never contains NULL.
> Either it's the pointer to the fd (with whatever FDPUT_xyz bits) or
> it's an error value. I don't know if that was your plan.

Definitely.  Mixing NULL and ERR_PTR() for error reporting is a bloody
bad idea; we *do* have functions that may legitimately return both,
but there NULL does not serve as an error indicator - ->lookup()
is one such case (ERR_PTR() on error, NULL - success, use dentry
passed to ->lookup(), non-NULL - a different dentry had to be
spliced in, use it instead).  IS_ERR_OR_NULL() is almost always
a sign of bad calling conventions, or the author being too lazy to
check what the calling conventions are... ;-/

> You can use similar tricks to then get the error value, ie
> 
>    #define fd_error(f) _Generic(f, \
>         struct fd: -EBADF, \
>         struct fd_err: PTR_ERR((f).word))
> 
> and now you can write code like
> 
>         if (fd_empty(x))
>                 return fd_error(x);
> 
> and it will automagically just DTRT, regardless of whether 'x' is a
> 'struct fd' or a 'struct fd_err'.

... except that there's a sizable fraction of those checks that return
something completely different instead of -EBADF ;-/  Userland ABI,
can't do anything about it...

> The same model goes for 'fdput()' - don't make people have to use two
> names and pointlessly state the type.

Maybe, but that depends upon the amount of explicit calls of either
left when we are done.  Conversions to CLASS(fd) eliminate a lot
of those...

> The bad old days where people were convinced that "Hungarian notation"
> was a good thing were bad. Let's leave them behind completely.
> 
> Worth it? Who knows.. But I like your type-based approach, and I think
> that _Generic() thing would fit very very with it.

I'm fine with that for fd_empty(); for the rest... let's see how the
things look like at the end of that series.