Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/35611 )
Change subject: cli_classic.c: Tidy up some repeated handling patterns into funcs ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/flashrom/+/35611/2/cli_classic.c File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/35611/2/cli_classic.c@74 PS2, Line 74: static void cli_classic_abort_usage(const char *msg)
Minor: Is there some sort of "noreturn" attribute for functions like this one? I guess it could be u […]
Sounds a bit unrelated to this change, maybe it should go into the compiler warn cleanups I saw since I am not introducing this function here.
https://review.coreboot.org/c/flashrom/+/35611/2/cli_classic.c@82 PS2, Line 82: int operation_specified
Does this work? AFAIK, this parameter gets passed by value, so incrementing it inside the function w […]
woops, done thanks! O_o
https://review.coreboot.org/c/flashrom/+/35611/2/cli_classic.c@82 PS2, Line 82: cli_classic_single_operation
A function name without a verb? Since functions do things, I would appreciate if the name had a verb […]
Done
https://review.coreboot.org/c/flashrom/+/35611/2/cli_classic.c@226 PS2, Line 226: if (layoutfile) { : cli_classic_abort_usage("Error: --layout specified more than once. Aborting.\n"); : } : if (ifd) { : cli_classic_abort_usage("Error: --layout and --ifd both specified. Aborting.\n"); : } : if (fmap) { : cli_classic_abort_usage("Error: --layout and --fmap-file both specified. Aborting.\n"); : }
I shall point out that this is quite redundant as well, so that you can't unsee it :)
I assume you mean the extra '{ .. }' here, done!