On Tue, Apr 25, 2023 at 09:28:54AM -0700, Linus Torvalds wrote: > Now, since they are inline functions, the code generation doesn't > really change (compilers are smart enough to not actually generate any > pointer stuff), but I prefer to make things like that expliict, and > have source code that matches the code generation. > > (Which is also why I do *not* endorse passing bigger structs by value, > because then the compiler will just pass it as a "pointer to a copy" > instead, again violating the whole concept of "source matches what > happens in reality") > > I think the above helper could be improved further with Al's > suggestion to make 'fd_publish()' return an error code, and allow the > file pointer (and maybe even the fd index) to be an error pointer (and > error number), so that you could often unify the error/success paths. > > IOW, I like this, and I think it's superior to my stupid original suggestion. We'd better collect the data on the current callers first. There are several patterns; I'm going through the old (fairly sparse) notes and the grep over the current tree right now, will post when I get through that. That's one area where we had a *lot* of recurring bugs - mostly of leak/double put variety. So we'd better have the calling conventions right wrt how easy it is to fuck up in failure exits. And we need to document the patterns/rules for each/reasons for choosing one over another. Note that there's also "set the file up, then get descriptor and either fd_install or fput, depending on get_unused_fd_flags() success"; sometimes it's the only approach (SCM_RIGHTS, for example), sometimes it's better than "get descriptor, set the file up, then either install or release descriptor", sometimes it's definitely worse (e.g. for O_CREAT it's a non-starter). It should be a deliberate choice.
