Patch Set 7:
Patch Set 7: Code-Review+1
Hi Nico, are we good to go on this particular one?
Absolutely not. Did you review the 5k lines patch? I would
do some testing on my own, probably add a few lines of C
that prints the database to compare before/after the patch.
Hi Nico,
Here are some notes about how this patch was made, and how I came to be confident that it only moves entries in the flashchips array rather than duplicating or omitting any.
The patch was generated from python script that parses entries out of flashchips.c. The script is currently out for review at https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/1687632. It may or may not be submitted.
I first tested that the script could read all the entries from flashchips.c and then rewrite the file without differences. I then used "rewrite --sort" to generate a version of flashchips.c with the entries sorted.
I've run a number of checks on the resulting file to ensure there are no differences in the flashchips array apart from the order of the entries.
The most comprehensive check involved a small awk script which constructs one line per entry in the flashchips array. I ran this script against the original flashchips.c and the sorted flashchips.c, to produce two files of 532 lines each:
$ awk '/^.{/ {x=$0; inside = 1} {if (inside) {x=x $0}} /^.}/ {inside = 0; print x}' orig_flashchips.c > orig_entries
$ awk '/^.{/ {x=$0; inside = 1} {if (inside) {x=x $0}} /^.}/ {inside = 0; print x}' sorted_flashchips.c > sorted_entries
$ wc -l {orig,sorted}_entries
532 orig_entries
532 sorted_entries
1064 total532 is the expected number. flashrom -L reports 519 chips, and there are also 11 entries for chips with known vendors but unknown names, 1 entry for "Unknown" "SFDP-capable chip" and 1 entry for "Programmer" "Opaque flash chip". (flashrom -L's counting takes place in print.c in print_supported_chips().)
By sorting and diffing each of {orig,sorted}_entries files, I was able to verify that for every line in the orig_entries file there was an identical line in the sorted_entries file.
This shows that the entries in the before and after versions of flashchips.c are textually identical, apart from their order.
> It's good that you ask btw. Your sudden appearance in the
> flashrom developers group in Gerrit already caused some
> friction. I don't know if any other member knows about
> that? I know that you are up to the task, it just caught
> me by surprise. And technically, with zero contributions
> to flashrom proper, you wouldn't even be in a reviewer
> group if we had one separate from coreboot's ;)
To view, visit change 33931. To unsubscribe, or for help writing mail filters, visit settings.