[M] Change in flashrom[main]: Replace all flashchips accesses with get functions

[Stefan Reinauer (Code Review)]

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan.

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75646?usp=email )

Change subject: Replace all flashchips accesses with get functions
......................................................................

Patch Set 5:

(15 comments)

Patchset:

PS5:
> I realise this patch was a intermediate step so the comments are just how it should probably look fo […]

Done

File flashchips.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/31223b9d_eaf0ba69 :
PS5, Line 28:  * Temporarily, this file is sorted alphabetically by vendor and name to
            :  * assist with merging the Chromium fork of flashrom.
> Can we remove this comment by now? (maybe in a separate patch). […]

Done

https://review.coreboot.org/c/flashrom/+/75646/comment/afb19e6f_1f31d463 :
PS5, Line 34: internal_
> I agree that "internal" almost always means internal programmer. It is flashromspeak :)  […]

Done

https://review.coreboot.org/c/flashrom/+/75646/comment/79a33fb2_da183bd0 :
PS5, Line 20844: get_flashchips(void)
> Where would the caller get the pointer to flashchip db? and why the caller needs to have the pointer […]

Any code that needs to access the supported flashchips would use this function instead of accessing the array directly. This allows to later load the database of supported flashchips dynamically rather than hard coding it in the binary.

https://review.coreboot.org/c/flashrom/+/75646/comment/b58c418a_34e7a568 :
PS5, Line 20849: unsigned int
> maybe so. it was unsigned int before, so i didn't change it. […]

Done

File flashrom.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/217b37be_b2fa90d2 :
PS5, Line 1099: return -1;
> Maybe you can add `msg_gerr` before returning. […]

Done

File ichspi.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/cc185715_06991b98 :
PS5, Line 1469: 	const struct flashchip *chip;
              : 
              : 	struct flashchip *flashchips=get_flashchips();
              : 
              : 	if (!flashchips)
              : 		return NULL;
> very clever. and doesn't check the return value. […]

Done

File libflashrom.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/ad9a70ce_cf7e3c48 :
PS5, Line 95: unsigned int
> shouldn't then i downstairs also be size_t? not against this, but it seems that is a larger consiste […]

Done

File print.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/e4138b87_af71dddb :
PS5, Line 56: const struct flashchip *chip;
> delete

This won't work because we walk the whole array twice and chip is modified the first time.

https://review.coreboot.org/c/flashrom/+/75646/comment/1cf56450_5101611f :
PS5, Line 62: flashchips
> chip

ditto

https://review.coreboot.org/c/flashrom/+/75646/comment/9a2e313e_0260c79b :
PS5, Line 65: chip = flashchips
> delete

ditto.

File print_wiki.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/a2bdff40_14e6d3e5 :
PS5, Line 294: flashchips
> get_flashchips()

Given that get_flashchips() might do more in the future, it doesn't make sense to optimize two lines of code saved by making two function calls in my opinion.

https://review.coreboot.org/c/flashrom/+/75646/comment/c12e89f3_e7f98e63 :
PS5, Line 310: flashchips
> get_flashchips()

ditto

File tests/selfcheck.c:

https://review.coreboot.org/c/flashrom/+/75646/comment/94c28057_65adc2ea :
PS5, Line 80: /* unused */
> But it is used two lines below?

Done

https://review.coreboot.org/c/flashrom/+/75646/comment/83be1367_b595402e :
PS5, Line 83: flashchips = get_flashchips();
            : 	flashchips_size = get_flashchips_size();
> Can this be immediately initialised? (and the same above). This is what non-test code is doing.

Done