Bora Guvendik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37940 )
Change subject: dram-spd: Remove free() ......................................................................
dram-spd: Remove free()
free() is not needed since the memory is not dynamically allocated.
Change-Id: I90659722aaca6ced1e1cbc3db4180b0811205e95 Signed-off-by: Bora Guvendik bora.guvendik@intel.com --- M src/vendorcode/cavium/bdk/libdram/dram-spd.c 1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/37940/1
diff --git a/src/vendorcode/cavium/bdk/libdram/dram-spd.c b/src/vendorcode/cavium/bdk/libdram/dram-spd.c index 894a060..8aec41c 100644 --- a/src/vendorcode/cavium/bdk/libdram/dram-spd.c +++ b/src/vendorcode/cavium/bdk/libdram/dram-spd.c @@ -105,7 +105,6 @@ int64_t data = bdk_twsix_read_ia(node, bus, address, i, 4, 1); if (data < 0) { - free(spd_buf); bdk_error("Failed to read SPD data at 0x%x\n", i + (bank << 8)); /* Restore the bank to zero */ if (bank)
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37940 )
Change subject: dram-spd: Remove free() ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37940 )
Change subject: dram-spd: Remove free() ......................................................................
Patch Set 1:
I think some of the static analysis tools are using it for modelling life times. But perhaps we should rename our malloc to something else instead?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37940 )
Change subject: dram-spd: Remove free() ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
I think some of the static analysis tools are using it for modelling life times. But perhaps we should rename our malloc to something else instead?
I'm not following. free() in this patch is just incorrect as the objects are allocated from bss section -- not malloc.
https://review.coreboot.org/c/coreboot/+/37940/1/src/vendorcode/cavium/bdk/l... File src/vendorcode/cavium/bdk/libdram/dram-spd.c:
https://review.coreboot.org/c/coreboot/+/37940/1/src/vendorcode/cavium/bdk/l... PS1, Line 63: static uint8_t spd_bufs[4 * 256]; /* FIXME(dhendrix): storage for SPD buffers, assume DDR4 */ This is where the allocation is coming from. Not malloc.
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37940 )
Change subject: dram-spd: Remove free() ......................................................................
dram-spd: Remove free()
free() is not needed since the memory is not dynamically allocated.
Change-Id: I90659722aaca6ced1e1cbc3db4180b0811205e95 Signed-off-by: Bora Guvendik bora.guvendik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37940 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/vendorcode/cavium/bdk/libdram/dram-spd.c 1 file changed, 0 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/vendorcode/cavium/bdk/libdram/dram-spd.c b/src/vendorcode/cavium/bdk/libdram/dram-spd.c index 894a060..8aec41c 100644 --- a/src/vendorcode/cavium/bdk/libdram/dram-spd.c +++ b/src/vendorcode/cavium/bdk/libdram/dram-spd.c @@ -105,7 +105,6 @@ int64_t data = bdk_twsix_read_ia(node, bus, address, i, 4, 1); if (data < 0) { - free(spd_buf); bdk_error("Failed to read SPD data at 0x%x\n", i + (bank << 8)); /* Restore the bank to zero */ if (bank)
-
Aaron Durbin (Code Review) -
Bora Guvendik (Code Review)
-
Patrick Georgi (Code Review)