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

Change in flashrom[master]: dummyflasher.c: Extract params processing into a separate function
by Angel Pons (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Namyoon Woo, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 ) Change subject: dummyflasher.c: Extract params processing into a separate function ...................................................................... Patch Set 2: (4 comments) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54748/comment/edc69df6_255d1d72 PS2, Line 971: free(status); : if (errno != 0 || status == endptr) { For another patch: this has an use-after-free bug, just like the one CB:46551 fixed. https://review.coreboot.org/c/flashrom/+/54748/comment/f2b3e60e_8c711f43 PS2, Line 975: return 1; This will result in `data->flashchip_contents` leaking. I'd move the initialisation out of init_data(), see comment below. https://review.coreboot.org/c/flashrom/+/54748/comment/56c51421_15cfc44e PS2, Line 1014: I would initialise `data->flashchip_contents` here: data->flashchip_contents = malloc(data->emu_chip_size); if (!data->flashchip_contents) { msg_perr("Out of memory!\n"); free(data); return 1; } https://review.coreboot.org/c/flashrom/+/54748/comment/50bfd870_76ed5651 PS2, Line 1035: emu_persistent_image Meh, this is leaking: CB:54905 -- To view, visit https://review.coreboot.org/c/flashrom/+/54748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87 Gerrit-Change-Number: 54748 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com> 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: Namyoon Woo <namyoon(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 10:53:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Don't leak `emu_persistent_image`
by Angel Pons (Code Review) 25 May '21

25 May '21

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/54905 ) Change subject: dummyflasher.c: Don't leak `emu_persistent_image` ...................................................................... dummyflasher.c: Don't leak `emu_persistent_image` Ensure `emu_persistent_image` doesn't end up leaking memory. Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M dummyflasher.c 1 file changed, 3 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/54905/1 diff --git a/dummyflasher.c b/dummyflasher.c index 2f39c98..8955254 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -646,9 +646,8 @@ write_buf_to_file(emu_data->flashchip_contents, emu_data->emu_chip_size, emu_data->emu_persistent_image); - free(emu_data->emu_persistent_image); - emu_data->emu_persistent_image = NULL; } + free(emu_data->emu_persistent_image); free(emu_data->flashchip_contents); } #endif @@ -1012,6 +1011,7 @@ if (read_buf_from_file(data->flashchip_contents, data->emu_chip_size, data->emu_persistent_image)) { msg_perr("Unable to read %s\n", data->emu_persistent_image); + free(data->emu_persistent_image); free(data->flashchip_contents); return 1; } @@ -1023,6 +1023,7 @@ dummy_init_out: if (register_shutdown(dummy_shutdown, data)) { + free(data->emu_persistent_image); free(data->flashchip_contents); free(data); return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/54905 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I76529973cefcc6a1472681e1f4da8239fcbf07a6 Gerrit-Change-Number: 54905 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: par_masters: Reshuffle to remove forward declarations
by Angel Pons (Code Review) 25 May '21

25 May '21

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54873 ) Change subject: par_masters: Reshuffle to remove forward declarations ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88e89992380195fee7c9de7ec57502ab980ec5df Gerrit-Change-Number: 54873 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 09:07:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52958 ) Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (2 comments) File buspirate_spi.c: https://review.coreboot.org/c/flashrom/+/52958/comment/a8785a3c_e81c42ef PS3, Line 58: unsigned char **bp_commbuf > does this actually need double indirection? I think so, because the pointer can change after realloc (and the size of the buffer can also change) - but please correct me if I am wrong. https://review.coreboot.org/c/flashrom/+/52958/comment/ca94ff25_803e2b85 PS3, Line 221: unsigned char *const bp_commbuf = bp_data->bp_commbuf; > perhaps just move this up a few lines to the functions preamble so that the branch line above can be […] I thought that buspirate_commbuf_grow can change the pointer, and I can have this local variable as constant pointer after buspirate_commbuf_grow. Both your comments have the same answer, depending on whether realloc can change the pointer? -- To view, visit https://review.coreboot.org/c/flashrom/+/52958 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Gerrit-Change-Number: 52958 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 25 May 2021 05:21:47 +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]: spi_master: Make use of new register_spi_master() API
by Anastasia Klimchuk (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54889 ) Change subject: spi_master: Make use of new register_spi_master() API ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/54889/comment/c232b097_fee78adf PS1, Line 14: TEST=builds > can you test raiden debug with a servo micro board since you have access to the hardware? Yes sure, let's leave it unresolved and I will reply to the comment after I test with hardware. -- To view, visit https://review.coreboot.org/c/flashrom/+/54889 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If72f54c28a95b402b3565fd14ea481f734e1c970 Gerrit-Change-Number: 54889 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 25 May 2021 05:12:58 +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]: programmer: Introduce default shutdown function
by Anastasia Klimchuk (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54890 ) Change subject: programmer: Introduce default shutdown function ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/54890/comment/ea8517a2_c0333805 PS1, Line 13: lot of par masters that : will use it in the nearest future. > lets rebase this once we have a few more users rather than too prematurely introducing it. […] Yes sure, my main goal was to get some comments, thank you for comment! In terms of more users, there are 10 out of 15 par masters which would use this. -- To view, visit https://review.coreboot.org/c/flashrom/+/54890 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b20717ba549e12edffbc5d1643ff064a4f0c517 Gerrit-Change-Number: 54890 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 25 May 2021 05:11:24 +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]: spi_master: Make use of new register_spi_master() API
by Edward O'Callaghan (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54889 ) Change subject: spi_master: Make use of new register_spi_master() API ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/54889/comment/a19905a3_dbbb0714 PS1, Line 14: TEST=builds can you test raiden debug with a servo micro board since you have access to the hardware? -- To view, visit https://review.coreboot.org/c/flashrom/+/54889 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If72f54c28a95b402b3565fd14ea481f734e1c970 Gerrit-Change-Number: 54889 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 05:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Separate shutdown from failed init cleanup
by Edward O'Callaghan (Code Review) 25 May '21

25 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 ) Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Gerrit-Change-Number: 52877 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 05:06:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52958 ) Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: Code-Review+1 (2 comments) File buspirate_spi.c: https://review.coreboot.org/c/flashrom/+/52958/comment/2867bc0d_94923a74 PS3, Line 58: unsigned char **bp_commbuf does this actually need double indirection? https://review.coreboot.org/c/flashrom/+/52958/comment/67990204_dd146a96 PS3, Line 221: unsigned char *const bp_commbuf = bp_data->bp_commbuf; perhaps just move this up a few lines to the functions preamble so that the branch line above can be made shorter by using the itermediate. -- To view, visit https://review.coreboot.org/c/flashrom/+/52958 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Gerrit-Change-Number: 52958 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 05:05:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Drop bp_ prefix for spi data struct members
by Edward O'Callaghan (Code Review) 25 May '21

25 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54886 ) Change subject: buspirate_spi.c: Drop bp_ prefix for spi data struct members ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54886 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia463e3f58497e4b10be072bdd29244f4868098e7 Gerrit-Change-Number: 54886 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 25 May 2021 05:01:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021