May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: cli_classic: fix memory leak
by Thomas Heijligen (Code Review) 15 May '22

15 May '22

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64351 ) Change subject: cli_classic: fix memory leak ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: This memory leak was introduced with CB:58738 cli_classic.c:680 `wp_region = strdup(optarg);` -- To view, visit https://review.coreboot.org/c/flashrom/+/64351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8520e302e9d63ed1215c5d9beb90a93fb52a91fe Gerrit-Change-Number: 64351 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sun, 15 May 2022 06:27:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util/shell.nix: Add libjaylink
by Thomas Heijligen (Code Review) 15 May '22

15 May '22

Attention is currently required from: Felix Singer. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64352 ) Change subject: util/shell.nix: Add libjaylink ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: PS2: Please mention older NixOS versions are eol. -- To view, visit https://review.coreboot.org/c/flashrom/+/64352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 Gerrit-Change-Number: 64352 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Sun, 15 May 2022 06:14:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util/shell.nix: Add libjaylink
by Paul Menzel (Code Review) 15 May '22

15 May '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64352 ) Change subject: util/shell.nix: Add libjaylink ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/64352/comment/1d3e59fb_c249d9e5 PS2, Line 9: remove the comment and : include the package libjaylink The hash in the beginning is easily overlooked, so maybe say “activate the line” or something similar, and add removing the comment last. -- To view, visit https://review.coreboot.org/c/flashrom/+/64352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 Gerrit-Change-Number: 64352 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sun, 15 May 2022 05:53:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: util/shell.nix: Add libjaylink
by Felix Singer (Code Review) 15 May '22

15 May '22

Attention is currently required from: Thomas Heijligen. Hello Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64352 to look at the new patch set (#2). Change subject: util/shell.nix: Add libjaylink ...................................................................... util/shell.nix: Add libjaylink NixOS 21.11 is out for a long time now. Thus, remove the comment and include the package libjaylink. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 --- M util/shell.nix 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/64352/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 Gerrit-Change-Number: 64352 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: util/shell.nix: Add libjaylink
by Felix Singer (Code Review) 15 May '22

15 May '22

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64352 ) Change subject: util/shell.nix: Add libjaylink ...................................................................... util/shell.nix: Add libjaylink NixOS 21.11 is out for a long time now. Thus, remove the comment and include the package. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 --- M util/shell.nix 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/64352/1 diff --git a/util/shell.nix b/util/shell.nix index d6ddf81..906e1fc 100644 --- a/util/shell.nix +++ b/util/shell.nix @@ -6,7 +6,7 @@ buildInputs = [ cmocka libftdi1 -# libjaylink # Will be added in NixOS 21.11 + libjaylink libusb1 meson ninja -- To view, visit https://review.coreboot.org/c/flashrom/+/64352 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I56e750831143a4e34be95ec111a37bb476abfe85 Gerrit-Change-Number: 64352 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: cli_classic: fix memory leak
by Joursoir (Code Review) 14 May '22

14 May '22

Joursoir has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64351 ) Change subject: cli_classic: fix memory leak ...................................................................... cli_classic: fix memory leak If parameter --wp-region is used and wp_region is allocated, free the last one at the exit of the application. Found-by: scan-build, clang v13.0.1 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Change-Id: I8520e302e9d63ed1215c5d9beb90a93fb52a91fe --- M cli_classic.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/64351/1 diff --git a/cli_classic.c b/cli_classic.c index 9543009..5cd05a7 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1100,6 +1100,8 @@ free(referencefile); free(layoutfile); free(pparam); + if (set_wp_region && wp_region) + free(wp_region); /* clean up global variables */ free((char *)chip_to_probe); /* Silence! Freeing is not modifying contents. */ chip_to_probe = NULL; -- To view, visit https://review.coreboot.org/c/flashrom/+/64351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8520e302e9d63ed1215c5d9beb90a93fb52a91fe Gerrit-Change-Number: 64351 Gerrit-PatchSet: 1 Gerrit-Owner: Joursoir <chat(a)joursoir.net> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Daniel Campello (Code Review) 13 May '22

13 May '22

Attention is currently required from: Richard Hughes, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... Patch Set 18: (6 comments) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/49643/comment/6849c788_b23f01ad PS17, Line 78: struct flashrom_update_progress { : enum flashrom_progress_stage stage; : size_t current; : size_t total; : void *user_data; : }; > should all this internal state be exposed to the user? That is how the user callback can make sense on where the progress is sitting at (i.e., current/total for stage) https://review.coreboot.org/c/flashrom/+/49643/comment/8b3e63cf_00e4b95d PS17, Line 84: struct flashrom_flashctx; > duplicates another forward declaration on line 205. Done https://review.coreboot.org/c/flashrom/+/49643/comment/a59672fa_e804352e PS17, Line 85: struct flashrom_update_progress *progress_state > X: I guess just document this signature better so it is clear why the passed copy is used and not th […] Done https://review.coreboot.org/c/flashrom/+/49643/comment/41408e9a_e3120fa9 PS17, Line 85: struct flashrom_update_progress *progress_state > X: I guess just document this signature better so it is clear why the passed copy is used and not th […] Done https://review.coreboot.org/c/flashrom/+/49643/comment/5a03cc1a_54a8ebfe PS17, Line 97: update > `s/update/set||init/` the func name made me dizzy. Done File libflashrom.c: https://review.coreboot.org/c/flashrom/+/49643/comment/456e6141_1a6703f8 PS17, Line 84: flashctx->progress_callback(flashctx, flashctx->progress_state); > Y: Because in theory this could become `flashctx->progress_callback(flashctx);` given the above in X […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 18 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <hughsient(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 13 May 2022 17:39:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Daniel Campello (Code Review) 13 May '22

13 May '22

Attention is currently required from: Richard Hughes, Daniel Campello, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Daniel Campello has uploaded a new patch set (#18) to the change originally created by Richard Hughes. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... libflashrom: Return progress state to the library user Projects using libflashrom like fwupd expect the user to wait for the operation to complete. To avoid the user thinking the process has "hung" or "got stuck" report back the progress complete of the erase, write and read operations. Include a test for the dummy spi25 device. TEST=./test_build.sh; ./flashrom -p lspcon_i2c_spi:bus=7 -r /dev/null Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Signed-off-by: Richard Hughes <richard(a)hughsie.com> Signed-off-by: Daniel Campello <campello(a)chromium.org> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M en29lv640b.c M include/flash.h M include/libflashrom.h M it87spi.c M jedec.c M libflashrom.c M libflashrom.map M linux_mtd.c M lspcon_i2c_spi.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/spi25.c M tests/tests.c M tests/tests.h 21 files changed, 171 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/49643/18 -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 18 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <hughsient(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Edward O'Callaghan (Code Review) 13 May '22

13 May '22

Attention is currently required from: Richard Hughes, Daniel Campello, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... Patch Set 17: (3 comments) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/49643/comment/48538fd7_4cfd86d3 PS17, Line 85: struct flashrom_update_progress *progress_state > I am a little confused on intended life-times here and why we have two copies of progress_state? We […] X: I guess just document this signature better so it is clear why the passed copy is used and not the one in flashctx if there is a reason? https://review.coreboot.org/c/flashrom/+/49643/comment/8beb72c8_44aa1600 PS17, Line 97: update `s/update/set||init/` the func name made me dizzy. File libflashrom.c: https://review.coreboot.org/c/flashrom/+/49643/comment/a2db2e8b_58eaa60c PS17, Line 84: flashctx->progress_callback(flashctx, flashctx->progress_state); Y: Because in theory this could become `flashctx->progress_callback(flashctx);` given the above in X. -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 17 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <hughsient(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 13 May 2022 03:14:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Return progress state to the library user
by Edward O'Callaghan (Code Review) 13 May '22

13 May '22

Attention is currently required from: Richard Hughes, Daniel Campello, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 ) Change subject: libflashrom: Return progress state to the library user ...................................................................... Patch Set 17: Code-Review+1 (3 comments) File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/49643/comment/732d4a3c_a3693258 PS17, Line 78: struct flashrom_update_progress { : enum flashrom_progress_stage stage; : size_t current; : size_t total; : void *user_data; : }; should all this internal state be exposed to the user? https://review.coreboot.org/c/flashrom/+/49643/comment/3919765d_a8294d14 PS17, Line 84: struct flashrom_flashctx; duplicates another forward declaration on line 205. https://review.coreboot.org/c/flashrom/+/49643/comment/0c3c4eef_469d8e74 PS17, Line 85: struct flashrom_update_progress *progress_state I am a little confused on intended life-times here and why we have two copies of progress_state? We have one explicitly in the functions operands and another copy implicitly within the flashctx. -- To view, visit https://review.coreboot.org/c/flashrom/+/49643 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c Gerrit-Change-Number: 49643 Gerrit-PatchSet: 17 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-CC: Richard Hughes <hughsient(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 13 May 2022 02:51:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2022