Attention is currently required from: Idwer Vollering, Angel Pons, Light.
Hello Keith Hui, build bot (Jenkins), Angel Pons, Anastasia Klimchuk, Keith Hui,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62761
to look at the new patch set (#6).
Change subject: board_enable.c: Fix dead assignment
......................................................................
board_enable.c: Fix dead assignment
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.
Change-Id: I7458b416a69fd5e2aa300ca49d1352b6074ad0bc
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M board_enable.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/62761/6
--
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: 6
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: 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-MessageType: newpatchset
Attention is currently required from: Nico Huber, Anastasia Klimchuk.
Light 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 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62747/comment/78c9ec41_e5f03f85
PS6, Line 16: zeroes
> This word "zeroes" can go to the previous line, no need to line break after "initialize them to".
Done
https://review.coreboot.org/c/flashrom/+/62747/comment/484300cb_82a6f32b
PS6, Line 16: or if allocating using malloc separately initialize them using a loop.
> Sorry I have just noticed: this specific case (separately initialize them using a loop) is not used […]
Done
--
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: 8
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 23 Mar 2022 08:03:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Light.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#8).
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
In flashrom_image_write variables curcontents and oldcontents are
dynamically allocated using malloc. These could remain uninitialized and
when later used in need_erase could result in undefined behaviour.
Similiar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/8
--
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: 8
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error
......................................................................
Patch Set 6:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/d4820da0_5d4e2a6d
PS6, Line 507: assert(j >= 12);
I am not sure, are we using assert in code? I am wondering what other people think.
Aarya, this is a question not to you :) Let's wait a bit and see what others think.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62764
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39
Gerrit-Change-Number: 62764
Gerrit-PatchSet: 6
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.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: Wed, 23 Mar 2022 07:58:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Light.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62747
to look at the new patch set (#7).
Change subject: flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
......................................................................
flashrom.c, sfdp.c: Initialize dynamically allocated memory using calloc
In flashrom_image_write variables curcontents and oldcontents are
dynamically allocated using malloc. These could remain uninitialized and
when later used in need_erase could result in undefined behaviour.
Similiar reasoning for variables hbuf, hdrs, tbuf in function
prob_spi_sfdp.
So allocate them using calloc to initialize them to zeroes or if
allocating using malloc separately initialize them using a loop.
Change-Id: I6b9269129968fb3b55b0d2a2e384c8a1aeba43ab
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
M sfdp.c
2 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/62747/7
--
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: 7
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Idwer Vollering, Angel Pons, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62761 )
Change subject: board_enable.c: Fix dead assignment
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Patchset:
PS4:
Angel, Keith, thank you so much for explanations, it was useful for me as well! My understanding the patch is all good? (tell me if I am wrong).
--
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: 4
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: 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: Wed, 23 Mar 2022 07:49:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Light.
Anastasia Klimchuk 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 6: Code-Review+1
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62747/comment/4293d8b2_394897f7
PS6, Line 16: zeroes
This word "zeroes" can go to the previous line, no need to line break after "initialize them to".
https://review.coreboot.org/c/flashrom/+/62747/comment/606e26d1_0b5c45ca
PS6, Line 16: or if allocating using malloc separately initialize them using a loop.
Sorry I have just noticed: this specific case (separately initialize them using a loop) is not used in the patch. This patch only calls calloc.
I can just remove the second part, "or if allocating using malloc ...".
Patchset:
PS6:
Apart from my two last comments, I think this is all good. And it fixes scan-build issues, which is very good!
But I wanted to check with Angel: this is on a similar topic like your patch CB:55269 , although your patch covers way more than this one. Do you approve this one?
--
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: 6
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Wed, 23 Mar 2022 07:42:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment