On Tue, Apr 06, 2021 at 02:15:01PM +0000, Al Viro wrote: > On Tue, Apr 06, 2021 at 03:22:05PM +0200, Christian Brauner wrote: > > > Why is a another function in charge of checking the return value of an > > initialization function. If something like path_init() fails why is the > > next caller responsible for rejecting it's return value and then we're > > passing that failure value through the whole function with if (!err) > > ladders but as I said it's mostly style preferences. > > Because otherwise you either need *all* paths leading to link_path_walk() > duplicate the logics (and get hurt whenever you miss one) or have "well, > in some cases link_path_walk() handles ERR_PTR() given to it, in some > cases its caller do" mess. > > > > > 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. > > > > Hm? Are you maybe overlooking path_init() which assigns straight into > > the variable declaration? Or are you referring to sm else? > > I'm referring to the fact that your diff is with an already modified path_lookupat() > _and_ those modifications have managed to introduce a bug your patch reverts. > No terminate_walk() paired with that path_init() failure, i.e. path_init() is > responsible for cleanups on its (many) failure exits... Note that the paste post the patch was just a doodle to illustrate the point not sm to review in earnest (I should probably comment prefix things like this with "untested".).
