[flashrom] [PATCH] Shorten some board enable related function names

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Aug 16 13:38:05 CEST 2011


Am 15.08.2011 23:42 schrieb Stefan Tauner:
> On Mon, 15 Aug 2011 22:40:58 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>   
>> Am 03.08.2011 13:40 schrieb Stefan Tauner:
>>     
>>> On Wed, 03 Aug 2011 09:02:52 +0200
>>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>>>
>>>   
>>>       
>>>> Suggestion:
>>>> struct board_pciid_enable -> struct board_match
>>>> board_match_coreboot_name() -> board_match_cbname()
>>>>
>>>> What do you think?
>>>>     
>>>>         
>>> looks fine imo.
>>> board_match_pci_card_ids could also be renamed to board_match_pci_ids
>>> and if you are at it please change the comment of it:
>>>   
>>>       
>> Done.
>>     
> oh my. now that i see the code, i think renaming the board enables to
> board_match is really a bad idea. mainly because we use "match" as verb
> in other places, but here as substantive.
> e.g.
> static const struct board_match *board_match_cbname
> a function that matches boards according to their cbname should return
> a board, not a "board match". it is just a question of taste, but i
> find it awful :)
>   

It's a trap!
The struct is there to match a board, so struct board_match is exactly
the right name for the _type_. The name of the function is another
thing, you could probably call it match_board_match_cbname, but that
would be silly, so we can either call it match_board_cbname or
board_match_cbname. And the name of the variable which stores the result
of the function call is even another thing, and there the name board
might be appropriate.


> the wiki table is named board_info hmhm maybe board_detail, but that's
> long.. :/
> what about just "board"?
> another way to mitigate "my" problem would be to no use match as a verb
> for the method names, but using "get" or "find" instead.
>   

"get" has the wrong semantic implication for the cbname matching, and
"find" as in board_find_cbname suffers from the same problem because
we're NOT interested in finding the cbname of the board, but rather in
looking for a board which matches the supplied cbname.


>>> aaaand if we change board_match_pci_card_ids we should also change
>>> pci_card_find to pci_dev_find or something like that... :)
>>>   
>>>       
>> Well, if we rename that one, we'd have to call it pci_dev_subsys_find,
>> and that's a net loss from the 80 column perspective, but it may indeed
>> clarify the code. Further input is appreciated.
>>     
> why do we have to? we find pci devs, not pci dev subsystems ;)
> hm maybe it should be find_pci_dev?
>   

pci_dev_find() looks for a device matching the supplied vendor+dev IDs.
pci_card_find() looks for a device matching the supplied
vendor+dev+subvendor+subdev IDs.

The name "card" was picked to allow differentiating between PCI devices
(cards) which share vendor+dev ID, but have different subvendor+subdev
IDs. That's pretty common for network cards where dozens of vendors use
the same network chip (and thus fixed vendor+dev ID), but completely
different PCBs.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the flashrom mailing list