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

Change in flashrom[master]: ich_descriptors.c: Initialize structures
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Angel Pons, Light. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 ) Change subject: ich_descriptors.c: Initialize structures ...................................................................... Patch Set 7: (1 comment) File ich_descriptors.c: https://review.coreboot.org/c/flashrom/+/62763/comment/d98164d3_25d1562c PS7, Line 1364: int ret = read_ich_descriptors_from_dump(dump, len, &cs, &desc); Is there anything in `desc` that is not filled when this call returns 0? If so, that would be a bug, I guess. -- To view, visit https://review.coreboot.org/c/flashrom/+/62763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2 Gerrit-Change-Number: 62763 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:40:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Idwer Vollering, Angel Pons, Light. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:33:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Light, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 ) Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62747/comment/6cc8def6_06dc6174 PS9, Line 11: when later used in need_erase could result in undefined behaviour. Can that happen generally, or only when there is another bug? If it would need another bug, initializing the memory to 0 could make it harder to detect that. For instance, if uninitialized data from `curcontents` could end up on a flash chip. That happening could be much more obvious with random values compared to constant 0. If we want to prepare for such bugs, we can choose something better than 0, for instance a repeated pattern like "flashrom bug! ". For `oldcontents` I'm not so sure. As it's used to compare to read data during verification, technically initializing it with pseudo- random values would be best to detect errors. But some static pat- tern should serve as well. -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 9 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:26:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: serprog.c: Avoid calling memcpy with NULL pointer arguments
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, Light. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62726 ) Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments ...................................................................... Patch Set 12: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62726 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b Gerrit-Change-Number: 62726 Gerrit-PatchSet: 12 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:16:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom.c: Fix unintialized value passed to function
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Felix Singer, Paul Menzel, Angel Pons, Light. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62725 ) Change subject: libflashrom.c: Fix unintialized value passed to function ...................................................................... Patch Set 15: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62725/comment/f1311a92_f47d4d95 PS15, Line 11: value > Replace "value" with "variable" IMO, "variable is passed" would sound like we're passing a reference to the variable (i.e. call-by-reference which doesn't exist in C). -- To view, visit https://review.coreboot.org/c/flashrom/+/62725 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacbd7bf9cdf897cc2a732c1dc6568845a4ab804d Gerrit-Change-Number: 62725 Gerrit-PatchSet: 15 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:13:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: pony_spi.c: Fix memory leak in function pony_init_spi
by Nico Huber (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Paul Menzel, Angel Pons, Light. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62724 ) Change subject: pony_spi.c: Fix memory leak in function pony_init_spi ...................................................................... Patch Set 10: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7910db94f63693e7f131836d4963e88cfdbec301 Gerrit-Change-Number: 62724 Gerrit-PatchSet: 10 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 13:08:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ich_descriptors.c: Initialize structures
by Felix Singer (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Nico Huber, Angel Pons, Light. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62763 ) Change subject: ich_descriptors.c: Initialize structures ...................................................................... Patch Set 7: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62763/comment/aba660bf_7a686d9c PS7, Line 9: Initialize structures so that they do not produce garbage nit (not blocking): Expand to 72 chars. -- To view, visit https://review.coreboot.org/c/flashrom/+/62763 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e5bc84c6199a0731db0a9c8ef56f1215686dab2 Gerrit-Change-Number: 62763 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 09:44:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Felix Singer (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Idwer Vollering, Angel Pons, Light. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 09:43:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: board_enable.c: Fix dead assignment
by Felix Singer (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Idwer Vollering, Angel Pons, Light. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 ) Change subject: board_enable.c: Fix dead assignment ...................................................................... Patch Set 7: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62761/comment/a324319a_872c6081 PS7, Line 7: Fix dead assignment Well, actually this doesn't fix something. How about: Remove unnecessary assignment https://review.coreboot.org/c/flashrom/+/62761/comment/2b2ec541_d8afd89b PS7, Line 9: In function board_asus_p3b_f there were two consecutive lines which : modified the value of variable b -- b=INB(0x80);b=INB(smbba);. Since the : value of b is not used after first assignment I have removed that : assignment. If you want to comment on code in a commit message, I would format this as I did below to make it more readable. Also, I slightly adjusted the last sentence. In function board_asus_p3b_f, there were two consecutive lines which modified the value of variable b // Do something with b b = INB(0x80); b = INB(smbba); // Do something with b Since the value of b is not used after first assignment, remove the first assignment. -- To view, visit https://review.coreboot.org/c/flashrom/+/62761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc Gerrit-Change-Number: 62761 Gerrit-PatchSet: 7 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 09:39:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
by Felix Singer (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Nico Huber, Light, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62747 ) Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/62747/comment/a9708189_8de20eeb PS9, Line 12: Similiar Similar -- To view, visit https://review.coreboot.org/c/flashrom/+/62747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab Gerrit-Change-Number: 62747 Gerrit-PatchSet: 9 Gerrit-Owner: Light <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Light <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 24 Mar 2022 09:29:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022