On Tue, 16 Aug 2011 13:38:05 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 15.08.2011 23:42 schrieb Stefan Tauner: > > On Mon, 15 Aug 2011 22:40:58 +0200 > > Carl-Daniel Hailfinger <[email protected]> wrote: > > > > > >> Am 03.08.2011 13:40 schrieb Stefan Tauner: > >> > >>> On Wed, 03 Aug 2011 09:02:52 +0200 > >>> Carl-Daniel Hailfinger <[email protected]> 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 <[email protected]> -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
