Attention is currently required from: Lance Zhao, Nico Huber, Maxim Polyakov, Johnny Lin, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35919 )
Change subject: inteltool: Allow to set cores range for MSRs dump ......................................................................
Patch Set 13:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/35919/comment/6f17d076_828f3e51 PS13, Line 11: intertool typo: inte*l*tool
File util/inteltool/cpu.c:
https://review.coreboot.org/c/coreboot/+/35919/comment/a49a1b1b_f5902d0d PS13, Line 2300: range_end != (unsigned int)(-1) uh... Use UINT_MAX?
File util/inteltool/inteltool.c:
https://review.coreboot.org/c/coreboot/+/35919/comment/a78d5b48_9b61eb22 PS13, Line 520: range How about naming this option `cpu-range`? This should avoid name clashes if anyone wants to add another range option in the future.
https://review.coreboot.org/c/coreboot/+/35919/comment/6d02f9a2_49478d7b PS13, Line 590: (unsigned int)(-1) UINT_MAX
https://review.coreboot.org/c/coreboot/+/35919/comment/62a5d416_2bcce9a8 PS13, Line 666: if (!dump_coremsrs) { AFAIUI, this will bail out if --range is specified before -M. I'd move this check out of the argument parsing loop.
https://review.coreboot.org/c/coreboot/+/35919/comment/f2635d1d_c9a686f3 PS13, Line 671: errno = 0; : if (strlen(optarg) == 0 || errno) { What do you use `errno` for here?
https://review.coreboot.org/c/coreboot/+/35919/comment/6858872a_6df53dee PS13, Line 676: sscanf(optarg, "%u-%u", &cores_range_start, &cores_range_end);
The sscanf function returns the value of the macro EOF if an input failure occurs before any conversion. Otherwise, the sscanf function returns the number of input items assigned, which can be fewer than provided for, or even zero, in the event of an early matching failure.
Please handle this function's return value.
https://review.coreboot.org/c/coreboot/+/35919/comment/a9f0a2ca_2c2cf4ac PS13, Line 681: } else if (cores_range_end == (unsigned int)(-1)) { Please use the return value of `sscanf()` to handle this instead of repeating a magic number.