Nico Huber has posted comments on this change. ( https://review.coreboot.org/23022 )
Change subject: make args for -r/-w/-v non-positional and optional ......................................................................
Patch Set 7: Code-Review-1
(1 comment)
It's moving into the right direction. But I'd still like to have that discussion about CrOS interface compatibility. If we decide to uncon- ditionally keep that (over possibly better interface choices), that's a very influental decision for the project. And shouldn't be made in a review.
Just imagine how fast this change could have been done for flashrom proper if we'd just kept the idea (per region files) and implemented it consistently with good old flashrom quality. I'd estimate a third of the time we already spent so far for gathering old commits, poli- shing, discussing.
https://review.coreboot.org/#/c/23022/7/cli_classic.c File cli_classic.c:
https://review.coreboot.org/#/c/23022/7/cli_classic.c@347 PS7, Line 347: char ** argv_copy = malloc((argc + 1) * sizeof(*argv_copy)); I'm not fully awake yet. Though, can't you just use
argv + optind
instead of a copy?
Either way you don't seem to reset `optind`, so the copied `argv[j]` with `j < optind` won't be considered by getopt_long() anyway. Which is just fine. But makes this code a hell to follow...
-
Nico Huber (Code Review)