Attention is currently required from: Matt DeVillier.
Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/85160?usp=email )
Change subject: cli_classic: Add option to use first detected chip if multiple found ......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2: Comment to the patch in general:
1) this needs update of the manpage, the source is at doc/classic_cli_manpage.rst This source will generate manpage, and also a webpage https://flashrom.org/classic_cli_manpage.html We have doc how to update docs: https://flashrom.org/how_to_add_docs.html
2) this needs to be added to the doc of Recent Development doc/release_notes/devel.rst Now is the special time we prepare new release, so devel.rst is being modified in CB:85156 Given that you have another patch, which also changed command line options, perhaps you can do a separate patch with both features explained? But keep in mind there is CB:85156 in flight
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/85160/comment/09288e46_0b7c71b5?usp... : PS2, Line 133: " --use-first-chip in cases where multiple chips are detected, use the first one found\n" This is potentially dangerous option: because same IDs can be not only between the chips in the same family. It could be same IDs for the chips in unrelated families, and even from different vendors. This feature can result in flashrom thinking it is processing an entirely different chip, not the one it actually is.
Usually in such situation we want to protect the users from accidentally using the feature. You can never fully protect, but at least to some extent. Basically, we want to only the people who know exactly what they are doing, to use the feature.
The approach used in the similar cases in the past was: 1) make default option safe (you did this already) 2) make the option name very scary, so that no one uses it out of curiosity
Existing examples of 2) are options such as `allow_brick` and `force_I_want_a_brick`
This comment is about naming: we need to come up a name that's scary enough. I will be thinking about it on the background, but maybe you can think too :) If you have ideas, please tell!