Taku Izumi wrote:
> Hi Eike
>
> >> +static int has_owner_file(struct pci_slot *pci_slot)
> >> +{
> >> + struct hotplug_slot *slot = pci_slot->hotplug;
> >> + if ((!slot) || (!slot->ops))
> >> + return -ENODEV;
> >> + if (slot->ops->owner)
> >> + return 0;
> >> + return -ENOENT;
> >> +}
> >
> > Given the name of the function I would expect to return it either 0 (no
> > slot there) or 1 (yes, there is one). On things like slot being NULL I
> > would also expect an error code. Now with the given name you would do
> > something like:
> >
> > if (!has_owner_file(foo))
> >
> > which is very unintuitive. From just reading over that code it would mean
> > exactly the opposite of what is expected.
> >
> > I see that the other functions around have the same interface so you are
> > not to blame for this, but ... well, understanding this expression needs
> > a look into the implementation of the has_foo() function. It also looks
> > as the error code is never checked for, so this function might really
> > return either 0 or 1 or bool.
>
> You are exactly right. I think the function whose name is "can_xxx" or
> "is_xxx" or "has_xxx"?should return boolean value and non zero meas True.
> However in order to maintain consistency with other functions, please
> overlook it.
As I said, you are not to blame for this ;) Maybe someone should cook up
against -next to clean up that once and forever?
Eike
Attachment:
signature.asc
Description: This is a digitally signed message part.
