Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33214
Change subject: drivers/amd/agesa: Assert that StdHeader is non-null ......................................................................
drivers/amd/agesa: Assert that StdHeader is non-null
Coverity believes there is a path where StdHeader is possibly null. This *should* be incorrect, since the header is actually initialized through the module dispatch framework, though Coverity can't see it due to the extensive type-punning. However, the control flow is so dizzingly complicated that I'm not even completely sure, so adding an extra assert to be careful won't hurt anyway.
Change-Id: If3d7c5d5c5bba846e7453b3dbc824e2208d749fb Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1379932 --- M src/drivers/amd/agesa/state_machine.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/33214/1
diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c index c8529c5e..750d192 100644 --- a/src/drivers/amd/agesa/state_machine.c +++ b/src/drivers/amd/agesa/state_machine.c @@ -272,7 +272,7 @@
/* Must call the function buffer was allocated for.*/ AMD_CONFIG_PARAMS *StdHeader = aip.NewStructPtr; - ASSERT(StdHeader->Func == func); + ASSERT(StdHeader != NULL && StdHeader->Func == func);
if (CONFIG(AGESA_EXTRA_TIMESTAMPS) && task.ts_entry_id) timestamp_add_now(task.ts_entry_id);
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33214 )
Change subject: drivers/amd/agesa: Assert that StdHeader is non-null ......................................................................
Patch Set 1: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33214 )
Change subject: drivers/amd/agesa: Assert that StdHeader is non-null ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33214 )
Change subject: drivers/amd/agesa: Assert that StdHeader is non-null ......................................................................
drivers/amd/agesa: Assert that StdHeader is non-null
Coverity believes there is a path where StdHeader is possibly null. This *should* be incorrect, since the header is actually initialized through the module dispatch framework, though Coverity can't see it due to the extensive type-punning. However, the control flow is so dizzingly complicated that I'm not even completely sure, so adding an extra assert to be careful won't hurt anyway.
Change-Id: If3d7c5d5c5bba846e7453b3dbc824e2208d749fb Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1379932 Reviewed-on: https://review.coreboot.org/c/coreboot/+/33214 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/amd/agesa/state_machine.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/drivers/amd/agesa/state_machine.c b/src/drivers/amd/agesa/state_machine.c index c8529c5e..750d192 100644 --- a/src/drivers/amd/agesa/state_machine.c +++ b/src/drivers/amd/agesa/state_machine.c @@ -272,7 +272,7 @@
/* Must call the function buffer was allocated for.*/ AMD_CONFIG_PARAMS *StdHeader = aip.NewStructPtr; - ASSERT(StdHeader->Func == func); + ASSERT(StdHeader != NULL && StdHeader->Func == func);
if (CONFIG(AGESA_EXTRA_TIMESTAMPS) && task.ts_entry_id) timestamp_add_now(task.ts_entry_id);