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.
Best regards,
Taku Izumi
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
