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

[Stefan Tauner]

On Tue, 16 Aug 2011 13:38:05 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> 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_.

if that would be its only purpose, it would contain identification
attributes only. last time i looked it had general info about boards
(vendor and board name) and board enable specific fields (phase,
max_rom etc). its previous name also suggests it is not just for
matching. ;)

> 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.

i think the reason for our difference of opinion is how we view the
table. for me it is a kind of database or container class. and we want
to query it for objects with certain properties.

in sane languages (supporting polymorphy) you could write that as
<container variable>.find_board(cbname). of course "match" would work
too, but as explained above i would like to use different terms just to
have a (mental) distinction.
 
> 
> >>> 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.

hm, ok

well... it is certainly an improvement over the current state and this
discussion wont lead to something more sane i feel, so think of it as
Acked-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at> 

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner