Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30865 )
Change subject: src/nb/intel/sandybridge/pcie.c: remove assert() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30865/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30865/1//COMMIT_MSG@9 PS1, Line 9: Use proper if() statements instead.
AFAIK, assertions are not a mechanism for handling run-time errors, but rather to prove the code is […]
They work in two ways:
1. Assertions specify a contract, e.g. in this case, don't call with `dev == NULL`. 2. When enabled at compile time, they abort program execution with an error message if the condition is false.
For the function in question, all the pointers should be set, otherwise the devicetree (structure in RAM not the file) would be inconsistent. By replacing the assertions with if's without printing a warning, you kill the possibility to detect inconsistencies with 2. Generally. Not that I'd expect somebody really to make use of the assertions here.
General rule: If an error condition can happen without the program being totally borked, always perform checks at runtime. If there'd be nothing you could do to fix things, an assertion might make sense.