Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40325 )
Change subject: Add writeprotect support ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/flashrom/+/40325/6/cli_classic.c File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/40325/6/cli_classic.c@718 PS6, Line 718: start = strtoul(argv[optind], &endptr, 0); : if (errno == ERANGE || errno == EINVAL || *endptr != '\0') { : msg_gerr("Error: value "%s" invalid\n", argv[optind]); : ret = 1; : goto out_shutdown; : } : : len = strtoul(argv[optind + 1], &endptr, 0); : if (errno == ERANGE || errno == EINVAL || *endptr != '\0') { : msg_gerr("Error: value "%s" invalid\n", argv[optind + 1]); I think we should avoid using `argv[optind]` and `argv[optind + 1]` to get the values here. It currently won't work at all since the two arguments will look like extraneous tokens and cause the program to abort on lines 408-409.
It's tricky since getopt_long() can't parse two arguments for a single flag. We could either: - Read the two tokens immediately after parsing "--wp-range" - Use getopt_long() to parse a single token with both values e.g. "--wp-range start,len"
https://review.coreboot.org/c/flashrom/+/40325/6/writeprotect.c File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/40325/6/writeprotect.c@851 PS6, Line 851: /* FIXME: Move to spi25.c if it's a JEDEC standard opcode */ Should this fn be moved?
https://review.coreboot.org/c/flashrom/+/40325/6/writeprotect.c@872 PS6, Line 872: /* FIXME: Move to spi25.c if it's a JEDEC standard opcode */ Ditto