Attention is currently required from: Lance Zhao, Nico Huber, Johnny Lin, Angel Pons, Patrick Rudolph. Maxim Polyakov 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 14:
(9 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/35919/comment/2eef6899_a678e5ac PS13, Line 11: intertool
typo: inte*l*tool
Done
Patchset:
PS14: Thanks for the review!
File util/inteltool/cpu.c:
https://review.coreboot.org/c/coreboot/+/35919/comment/602c23e9_ee9a729d PS13, Line 2300: range_end != (unsigned int)(-1)
uh... […]
Yes, it looks better, thanks!
File util/inteltool/inteltool.c:
https://review.coreboot.org/c/coreboot/+/35919/comment/6215553c_84eeb5ef PS13, Line 520: range
How about naming this option `cpu-range`? This should avoid name clashes if anyone wants to add anot […]
Ok, I agree, let's use --cpu-range.
https://review.coreboot.org/c/coreboot/+/35919/comment/0d1b19a1_ef2cf376 PS13, Line 590: (unsigned int)(-1)
UINT_MAX
Done
https://review.coreboot.org/c/coreboot/+/35919/comment/065070dd_1377c214 PS13, Line 666: if (!dump_coremsrs) {
AFAIUI, this will bail out if --range is specified before -M. […]
It seems to me that this check is unnecessary and there is no point in it. It just won't work if the -M option isn't set. This code block was removed in the last patch.
https://review.coreboot.org/c/coreboot/+/35919/comment/d12d21fc_008c1804 PS13, Line 671: errno = 0; : if (strlen(optarg) == 0 || errno) {
What do you use `errno` for here?
Sorry for the inattention. This is not necessary here.
https://review.coreboot.org/c/coreboot/+/35919/comment/f9f33a33_7eebbdaa 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 conve […]
Ok, this way is much better, thanks!
https://review.coreboot.org/c/coreboot/+/35919/comment/dbd1b948_e07eb3bc 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.
Done