March 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Makefile,meson.build: Fix dependency issues with raiden_debug_spi
by Edward O'Callaghan (Code Review) 29 Mar '21

29 Mar '21

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/51930 ) Change subject: Makefile,meson.build: Fix dependency issues with raiden_debug_spi ...................................................................... Makefile,meson.build: Fix dependency issues with raiden_debug_spi The Raiden debug external spi programmer need only depend on libusb and is unrelated to libpci. Correct meson and gnu make builds where meson had configuration in the incorrect section and gnu make artifactually disabled the build on Windows/MinGW which doesn't have libpci. BUG=none TEST=still continues to build. Change-Id: I2d6a8c33a2228abf006a9b278bcb7133765c7074 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile M meson.build 2 files changed, 1 insertion(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/51930/1 diff --git a/Makefile b/Makefile index 6d37d55..0cfff55 100644 --- a/Makefile +++ b/Makefile @@ -248,11 +248,6 @@ else override CONFIG_RAYER_SPI = no endif -ifeq ($(CONFIG_RAIDEN_DEBUG_SPI), yes) -UNSUPPORTED_FEATURES += CONFIG_RAIDEN_DEBUG_SPI=yes -else -override CONFIG_RAIDEN_DEBUG_SPI = no -endif ifeq ($(CONFIG_NIC3COM), yes) UNSUPPORTED_FEATURES += CONFIG_NIC3COM=yes else diff --git a/meson.build b/meson.build index d5895d8..cd4a586 100644 --- a/meson.build +++ b/meson.build @@ -99,6 +99,7 @@ config_digilent_spi = false config_developerbox_spi = false config_pickit2_spi = false + config_raiden_debug_spi = false endif # some programmers require libpci @@ -113,7 +114,6 @@ config_atavia = false config_drkaiser = false config_gfxnvidia = false - config_raiden_debug_spi = false config_internal = false config_it8212 = false config_nic3com = false -- To view, visit https://review.coreboot.org/c/flashrom/+/51930 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d6a8c33a2228abf006a9b278bcb7133765c7074 Gerrit-Change-Number: 51930 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: meson: remove rayer_spi dependency on libpci
by Edward O'Callaghan (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51929 ) Change subject: meson: remove rayer_spi dependency on libpci ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51929 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7206a69d031c9bba9475a9e6769f6ef35701379 Gerrit-Change-Number: 51929 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Mar 2021 23:27:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 ) Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: Even better news: now after adding includes in meson.build (my previous patchset) I discovered that unittest.h is not needed anymore in test files! So I am removing it from here. And now ninja test warnings are fixed - because there are no re-definitions anymore. This looks like a happy ending: only one header, included in build file, no need to touch the code, all files are covered by memory checks when they are built for test environment. I did all the testing for this last patchset, as before: nm files built for test vs for real, nm test files, running tests. Also introducing memory leak and verifying that test catches it (examples described in commit message). -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 6 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Mar 2021 23:25:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51243 to look at the new patch set (#6). Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Enable dynamic memory allocation checks for cmocka unit tests This commit enables the feature and makes changes to existing files and tests. I am writing more new tests with this. Commit includes tests/flashrom.c because after enabling memory checks the test started to fail (it used to leak memory indeed). If you are wondering how to verify it works (because at the moment all tests [still] pass so it’s not obvious that anything has changed), then for example: 1) Remove free’s in flashbuses_to_text_test_success test, and it will fail with message similar to this (line numbers from your local source) [ ERROR ] --- Blocks allocated... ../flashrom.c:1239: note: block 0x55f42304b640 allocated here ../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here ../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here ../flashrom.c:1239: note: block 0x55f42304b700 allocated here ../flashrom.c:1239: note: block 0x55f42304b780 allocated here ../flashrom.c:1239: note: block 0x55f42304bb00 allocated here ../flashrom.c:1239: note: block 0x55f42304b810 allocated here ERROR: flashbuses_to_text_test_success leaked 7 block(s) 2) Add char *temp = malloc just before return from strcat_realloc [ ERROR ] --- Blocks allocated... ../helpers.c:88: note: block 0x55a51307b6c0 allocated here ../helpers.c:88: note: block 0x55a51307b9e0 allocated here ERROR: strcat_realloc_test_success leaked 2 block(s) BUG=b:181803212 TEST=builds and ninja test nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o nm builddir/tests/flashrom_unit_tests.p/flashrom.c.o nm builddir/flashrom.p/flashrom.c.o Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build M tests/flashrom.c M tests/helpers.c M tests/meson.build A unittest_env.h 5 files changed, 86 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 6 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51243 to look at the new patch set (#5). Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Enable dynamic memory allocation checks for cmocka unit tests This commit enables the feature and makes changes to existing files and tests. I am writing more new tests with this. Commit includes tests/flashrom.c because after enabling memory checks the test started to fail (it used to leak memory indeed). If you are wondering how to verify it works (because at the moment all tests [still] pass so it’s not obvious that anything has changed), then for example: 1) Remove free’s in flashbuses_to_text_test_success test, and it will fail with message similar to this (line numbers from your local source) [ ERROR ] --- Blocks allocated... ../flashrom.c:1239: note: block 0x55f42304b640 allocated here ../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here ../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here ../flashrom.c:1239: note: block 0x55f42304b700 allocated here ../flashrom.c:1239: note: block 0x55f42304b780 allocated here ../flashrom.c:1239: note: block 0x55f42304bb00 allocated here ../flashrom.c:1239: note: block 0x55f42304b810 allocated here ERROR: flashbuses_to_text_test_success leaked 7 block(s) 2) Add char *temp = malloc just before return from strcat_realloc [ ERROR ] --- Blocks allocated... ../helpers.c:88: note: block 0x55a51307b6c0 allocated here ../helpers.c:88: note: block 0x55a51307b9e0 allocated here ERROR: strcat_realloc_test_success leaked 2 block(s) BUG=b:181803212 TEST=builds and ninja test nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o nm builddir/tests/flashrom_unit_tests.p/flashrom.c.o nm builddir/flashrom.p/flashrom.c.o Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build M tests/flashrom.c M tests/helpers.c M tests/meson.build A unittest_env.h 5 files changed, 85 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: meson: fix dependency on raw access
by Daniel Campello (Code Review) 29 Mar '21

29 Mar '21

Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 ) Change subject: meson: fix dependency on raw access ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/51577/comment/7e84b8d7_6a6bff1e PS3, Line 280: need_raw_access = true > I've just read through `rayer_spi.c`. It's a very simple driver using an LPT […] Sounds good. Sent review.coreboot.org/c/flashrom/+/51929 -- To view, visit https://review.coreboot.org/c/flashrom/+/51577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Gerrit-Change-Number: 51577 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Mar 2021 22:58:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: remove rayer_spi dependency on libpci
by Daniel Campello (Code Review) 29 Mar '21

29 Mar '21

Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/51929 ) Change subject: meson: remove rayer_spi dependency on libpci ...................................................................... meson: remove rayer_spi dependency on libpci This change removes the build dependency on libpci for config_rayer_spi it also makes sure that dependency on raw_access is maintained. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: If7206a69d031c9bba9475a9e6769f6ef35701379 --- M meson.build 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/51929/1 diff --git a/meson.build b/meson.build index d5895d8..c755ced 100644 --- a/meson.build +++ b/meson.build @@ -123,7 +123,6 @@ config_nicnatsemi = false config_nicrealtek = false config_ogp_spi = false - config_rayer_spi = false config_satamv = false config_satasii = false endif @@ -279,6 +278,7 @@ if config_rayer_spi srcs += 'rayer_spi.c' config_bitbang_spi = true + need_raw_access = true cargs += '-DCONFIG_RAYER_SPI=1' endif if config_satamv -- To view, visit https://review.coreboot.org/c/flashrom/+/51929 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7206a69d031c9bba9475a9e6769f6ef35701379 Gerrit-Change-Number: 51929 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: meson: fix dependency on raw access
by Nico Huber (Code Review) 29 Mar '21

29 Mar '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51577 ) Change subject: meson: fix dependency on raw access ...................................................................... Patch Set 3: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/51577/comment/f6de76e5_91b3b3f6 PS3, Line 280: need_raw_access = true > You are right! I did not notice that the Makefile does not request libpci. […] I've just read through `rayer_spi.c`. It's a very simple driver using an LPT (parallel) port. No sign of PCI. A follow up would be nice. Feel free to also remove the `config_rayer_spi = false` for libpci above (line ~124). -- To view, visit https://review.coreboot.org/c/flashrom/+/51577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Gerrit-Change-Number: 51577 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Mar 2021 22:32:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51243 ) Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Patch Set 4: (2 comments) Patchset: PS2: > Nico, sorry for a delay, I just needed some time to think about it. […] Good news: I made it work! No need to include unittest_env.h in every file under test, they are now all covered. I inspected object files with nm, everything built for tests has malloc, free etc redefined, at the same time files built for real have normal memory functions. Specifically: nm builddir/tests/flashrom_unit_tests.p/.._flashrom.c.o Has _test_calloc _test_free _test_malloc _test_realloc nm builddir/flashrom.p/flashrom.c.o Has calloc free malloc realloc Side effect is that now running ninja test produces warnings (but seems fine, especially given this is exactly what’s happening). Example of warning (there are 4 of them) In file included from ../tests/flashrom.c:19: ../tests/unittest.h:27: warning: "malloc" redefined 27 | #define malloc test_malloc | In file included from <command-line>: ../unittest_env.h:46: note: this is the location of the previous definition 46 | #define malloc(size) _test_malloc(size, __FILE__, __LINE__) However the same approach did not work for test files :( so I am still including unittest.h into a test file, not from tests/meson.build. Which I hope is less of a problem, since this is now contained in tests/ directory. Patchset: PS4: I have the first attempt to include unittest_env.h from command line for all files under test. This solution is working, but comments are very welcome! -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 29 Mar 2021 21:28:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Enable dynamic memory allocation checks for cmocka unit tests
by Anastasia Klimchuk (Code Review) 29 Mar '21

29 Mar '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan. Hello build bot (Jenkins), Nico Huber, Daniel Kurtz, David Hendricks, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51243 to look at the new patch set (#4). Change subject: Enable dynamic memory allocation checks for cmocka unit tests ...................................................................... Enable dynamic memory allocation checks for cmocka unit tests This commit enables the feature and makes changes to existing files and tests. I am writing more new tests with this. Commit includes tests/flashrom.c because after enabling memory checks the test started to fail (it used to leak memory indeed). If you are wondering how to verify it works (because at the moment all tests [still] pass so it’s not obvious that anything has changed), then for example: 1) Remove free’s in flashbuses_to_text_test_success test, and it will fail with message similar to this (line numbers from your local source) [ ERROR ] --- Blocks allocated... ../flashrom.c:1239: note: block 0x55f42304b640 allocated here ../flashrom.c:1239: note: block 0x55f42304b5c0 allocated here ../flashrom.c:1239: note: block 0x55f42304b3d0 allocated here ../flashrom.c:1239: note: block 0x55f42304b700 allocated here ../flashrom.c:1239: note: block 0x55f42304b780 allocated here ../flashrom.c:1239: note: block 0x55f42304bb00 allocated here ../flashrom.c:1239: note: block 0x55f42304b810 allocated here ERROR: flashbuses_to_text_test_success leaked 7 block(s) 2) Add char *temp = malloc just before return from strcat_realloc [ ERROR ] --- Blocks allocated... ../helpers.c:88: note: block 0x55a51307b6c0 allocated here ../helpers.c:88: note: block 0x55a51307b9e0 allocated here ERROR: strcat_realloc_test_success leaked 2 block(s) BUG=b:181803212 TEST=builds and ninja test Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M meson.build M tests/flashrom.c M tests/helpers.c M tests/meson.build A tests/unittest.h A unittest_env.h 6 files changed, 119 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/51243/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/51243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c6b6b8dc17aaee28640e3fca3d1fc9f7feabf5f Gerrit-Change-Number: 51243 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)google.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-CC: Victor Ding <victording(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2021