On Wed, Dec 27, 2023 at 07:14:50AM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 27, 2023 at 01:09:03AM +0000, Shinichiro Kawasaki wrote:
> > On Dec 26, 2023 / 18:34, Bjorn Helgaas wrote:
> > > On Mon, Dec 25, 2023 at 06:26:56PM +0900, Shin'ichiro Kawasaki wrote:
...
> > > > +static int p2sb_valid_resource(struct resource *res)
> > > > +{
> > > > + return res->flags ? 0 : -ENOENT;
> > > > +}
> > >
> > > This got worse because it's *named* like a boolean, but the return
> > > value can't be used like a boolean, which makes callers really hard to
> > > read, e.g., this:
> > >
> > > if (p2sb_valid_resource(res))
> > > /* do something */
> > >
> > > does exactly the opposite of what the reader expects.
> > >
> > > I see that you want to use this -ENOENT return value in the callers:
> > I have to admit that the function name meaning is opposite... When I
> > followed Andy's idea to make the function to return -ENOENT, I
> > should have renamed the function to not cause the confusion.
>
> Oh, sorry, I hadn't seen that idea. But your v4 looks good to me (at
> least this part; I didn't look carefully at the whole patch :) ).
Bjorn is the maintainer, I'm pretty much okay with his suggestions.
I will be off for a month, so feel free to use my
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
as starting from v4 it LGTM.
--
With Best Regards,
Andy Shevchenko
