Change in flashrom[master]: programmer.h,chipset_enable.c: Make penable fn ptr more descript
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52364 )
Change subject: programmer.h,chipset_enable.c: Make penable fn ptr more descript ......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/52364/comment/d4b4bb2f_d3ffff53 PS1, Line 7: Make penable cb more descript
It's not a callback anyway. It's a function pointer used for […]
Reading a bit too deeply into such a small change are we not, I guess I went a bit far with such detail in the commit message myself. I only said "cb" to try and keep the short line short. I went with 'fn ptr' instead as Nico points out its more semantically correct to say its a function pointer given the control flow.
https://review.coreboot.org/c/flashrom/+/52364/comment/49850b8d_c0067490 PS1, Line 15: use
typo use*d*
Done
File programmer.h:
https://review.coreboot.org/c/flashrom/+/52364/comment/3e6539c6_6daceff9 PS1, Line 228: enable_flash_xxx
How about `run` or anything else that doesn't repeat `enable`.
I think I much prefer the current pattern as "_xxx" as, in programming 'xxx' is typically recognised as a find-and-replace pattern and thus, it makes it super easy to identify that `enable_flash_sis5511()` is a concrete realisation for an example.
Any other name incurs more mental indirection which entirely is the motivation of the change to remove. It is easy for us given that we are familiar with the code however imagine someone who has never looked at Flashrom before, see's this struct, greps the field name or struct identifier. The developer would very quickly conclude the connection with the consistent naming without any more effort than a pattern match.
TL;DR the mundane `enable_flash_xxx` identifier leads to both consistent naming and minimal mental indirection with maximal grep friendliness.
-
Edward O'Callaghan (Code Review)