Attention is currently required from: Raul Rangel, Martin L Roth, Jon Murphy, Arthur Heymans, Tim Van Patten, Angel Pons, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74736 )
Change subject: util/sconfig: Include optional maiboard chip header ......................................................................
Patch Set 3:
(2 comments)
File util/sconfig/main.c:
https://review.coreboot.org/c/coreboot/+/74736/comment/9fb6c9b7_8477c265 PS3, Line 1402: &chip_h[strlen("src/")]
I don't understand this indexing into `chip_h`. […]
Mainboard path is got from devicetree path which in turn always contains "src/". I thought about adding check for src/ substring and am still open to it. But if the path is going to contain src/ always, that check might be redundant.
https://review.coreboot.org/c/coreboot/+/74736/comment/0df70f80_02dba5c7 PS3, Line 1405: free(mainboard_path);
This is potentially a double-free, since `mainboard_path` does not always allocate its own memory (r […]
I think the memory location immediately preceding the allocated/freed address will have some metadata to help with freeing. From that standpoint, even if mainboard_path is just referencing the address it is fine to use it during free. Note: mainboard_path is still pointing to the start of the allocated buffer (same as allocated in base_devtree_path/dir).