On Tue, Apr 06, 2021 at 02:35:05PM +0200, Christian Brauner wrote: > And while we're at it might I bring up the possibility of an additional > cleanup of how we currently call path_init(). > Right now we pass the return value from path_init() directly into e.g. > link_path_walk() which as a first thing checks for error. Which feels > rather wrong and has always confused me when looking at these codepaths. Why? > I get that it might make sense for reasons unrelated to path_init() that > link_path_walk() checks its first argument for error but path_init() > should be checked for error right away especially now that we return > early when LOOKUP_CACHED is set without LOOKUP_RCU. But you are making the _callers_ of path_init() do that, for no good reason. > thing especially in longer functions such as path_lookupat() where it > gets convoluted pretty quickly. I think it would be cleaner to have > something like [1]. The early exists make the code easier to reason > about imho. But I get that that's a style discussion. Your variant is a lot more brittle, actually. > @@ -2424,33 +2424,49 @@ static int path_lookupat(struct nameidata *nd, unsigned flags, struct path *path > int err; > > s = path_init(nd, flags); > - if (IS_ERR(s)) > - return PTR_ERR(s); Where has that come from, BTW? Currently path_lookupat() does no such thing.
