Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35364 )
Change subject: drivers/intel/fsp2_0: Add NULL pointer debug messages to UPD ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35364/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35364/1//COMMIT_MSG@9 PS1, Line 9: Check for a NULL pointer before displaying UPD information in case : the new area wasn't allocated. I am curious to understand the motivation behind this. Did you run into an issue with this? Looking at the call sequences, it doesn't look like you should ever get a NULL pointer in the places you are checking.
FSP_M --> Uses a storage on stack and creates a pointer to it FSP_S --> Uses xmalloc() which halts if allocation fails.
https://review.coreboot.org/c/coreboot/+/35364/1/src/drivers/intel/fsp2_0/up... File src/drivers/intel/fsp2_0/upd_display.c:
https://review.coreboot.org/c/coreboot/+/35364/1/src/drivers/intel/fsp2_0/up... PS1, Line 31: new Can this really be NULL? Looking at the call chain, it doesn't look like that is possible?
https://review.coreboot.org/c/coreboot/+/35364/1/src/drivers/intel/fsp2_0/up... PS1, Line 61: !fspm_new_upd Can this really be NULL? Looking at the call chain, it doesn't look like that is possible?
https://review.coreboot.org/c/coreboot/+/35364/1/src/drivers/intel/fsp2_0/up... PS1, Line 74: new If you plan to check new down in soc_display_fspm_upd_params(), then it will have to be checked here as well before calling soc_display_fspm_upd_params()
https://review.coreboot.org/c/coreboot/+/35364/1/src/drivers/intel/fsp2_0/up... PS1, Line 83: fsps_new_upd Same question as above about the possibility of this being NULL.