Attention is currently required from: Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63135 )
Change subject: linux_mtd: Allow to open parition by name
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
File linux_mtd.c:
https://review.coreboot.org/c/flashrom/+/63135/comment/5d1d5a8a_18298a6d
PS1, Line 464: extract_programmer_param("name");
> I thought it's already mutually exclusive as extract_programmer_param("name"); is in the else branch.
> I did not reuse the existing dev as a partition label might contain digits only. In that case you'd not know if the number is the MTD device or the label.
I see, thanks for explaining.
It's mutually exclusive at runtime but it won't complain if you provide both "name" and "dev". We don't check for other meaningless programmer params so that's fine.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63135
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68
Gerrit-Change-Number: 63135
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 28 Mar 2022 12:17:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Arthur Heymans.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63135 )
Change subject: linux_mtd: Allow to open parition by name
......................................................................
Patch Set 1:
(1 comment)
File linux_mtd.c:
https://review.coreboot.org/c/flashrom/+/63135/comment/3d3408b1_8c092e88
PS1, Line 464: extract_programmer_param("name");
> Would it make sense to use this path when strtol path is not taken so that no new parameter is neede […]
I thought it's already mutually exclusive as extract_programmer_param("name"); is in the else branch.
I did not reuse the existing dev as a partition label might contain digits only. In that case you'd not know if the number is the MTD device or the label.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63135
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I566b9809950cbf47fbb01ae89305641c4e259f68
Gerrit-Change-Number: 63135
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 28 Mar 2022 10:56:04 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
......................................................................
Patch Set 15:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62764/comment/a249c716_659ba924
PS12, Line 7: ich_descriptors.c: Assert unsigned types >=0 on to prevent underflow error
> The second part is left: to fit commit description into 72 chars, and it is 74 at the moment. […]
Done
--
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: 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: 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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 28 Mar 2022 10:45:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Light <aarya.chaumal(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62764
to look at the new patch set (#15).
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
......................................................................
ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow
Unsigned types show undefined behaviour if they are subtracted by a
value greater than their own (mostly it wraps to the max value). Using
this value for left shifting could be even more dangerous.
Change-Id: I5921cc571f3dca5188ca1973dba6ececbcbe2f39
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M ich_descriptors.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/62764/15
--
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: 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: 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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63130 )
Change subject: pony_spi.c: Extract out get_params to simplify init
......................................................................
Patch Set 1:
(1 comment)
File pony_spi.c:
https://review.coreboot.org/c/flashrom/+/63130/comment/7820752e_f4c5eabb
PS1, Line 164: free(data);
Why isn't data freed in the patch? Wouldn't this lead to memory leak.
--
To view, visit https://review.coreboot.org/c/flashrom/+/63130
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I364febc05c870683cbad114583762b0c006f4bac
Gerrit-Change-Number: 63130
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 28 Mar 2022 10:42:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons.
Light has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow error
......................................................................
Patch Set 14:
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/17cd4d7a_a2d36b05
PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++)
> Maybe these `12`'s should be a `#define MAX_NUM_REGIONS 12` or at the very least a constant after li […]
I don't think it's the max number of regions as num_regions can be > 12. Can you suggest any other name for the macro.
--
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: 14
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: 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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 28 Mar 2022 10:29:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Paul Menzel, Edward O'Callaghan, Angel Pons.
Light 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 14:
(1 comment)
File serprog.c:
https://review.coreboot.org/c/flashrom/+/62726/comment/24d80284_3d424861
PS14, Line 288: m
> `calloc(1, 1 + parmlen);` so that sp[1] is zeroed in the case of `(parmlen:=0, params:=NULL)`.
Does it help to calloc there?
--
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: 14
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: 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: Felix Singer <felixsinger(a)posteo.net>
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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 28 Mar 2022 09:55:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Light.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62764 )
Change subject: ich_descriptors.c: Ensure unsigned types >=0 on to prevent underflow error
......................................................................
Patch Set 14: Code-Review+1
(4 comments)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/62764/comment/332894d1_2a900d04
PS14, Line 501: for (j = 0; j < (size_t)min(num_regions, 12); j++)
Maybe these `12`'s should be a `#define MAX_NUM_REGIONS 12` or at the very least a constant after line 489.
https://review.coreboot.org/c/flashrom/+/62764/comment/2f096e3b_81b9061e
PS14, Line 505: for (j = 12; j < num_regions; j++)
ditto
https://review.coreboot.org/c/flashrom/+/62764/comment/31c31bf3_827fe455
PS14, Line 507: 12
ditto
https://review.coreboot.org/c/flashrom/+/62764/comment/1691e479_8c73d6e8
PS14, Line 508: 12
ditto
--
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: 14
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: 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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Mon, 28 Mar 2022 09:51:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Paul Menzel, Angel Pons, Light.
Edward O'Callaghan 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 14: Code-Review+1
(1 comment)
File serprog.c:
https://review.coreboot.org/c/flashrom/+/62726/comment/54a2e94b_ef70552f
PS14, Line 288: m
`calloc(1, 1 + parmlen);` so that sp[1] is zeroed in the case of `(parmlen:=0, params:=NULL)`.
--
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: 14
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: 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: Felix Singer <felixsinger(a)posteo.net>
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: Mon, 28 Mar 2022 09:40:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment