Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 )
Change subject: layout: Check return values for strdup in register_include_arg
......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70006/comment/b4a9e04f_3de5b2c2
PS2, Line 11:
> Explain the issues and the new code changes.
Done
File layout.c:
https://review.coreboot.org/c/flashrom/+/70006/comment/c801d890_332d18c9
PS2, Line 139: || file == NULL
> Just rethinking.. […]
Yes I forgot that indeed file is optional parameter. Please have a look at my latest version, whether it covers what we want? I read all your comments carefully.
--
To view, visit https://review.coreboot.org/c/flashrom/+/70006
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Gerrit-Change-Number: 70006
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 03 Dec 2022 09:32:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/70006
to look at the new patch set (#3).
Change subject: layout: Check return values for strdup in register_include_arg
......................................................................
layout: Check return values for strdup in register_include_arg
strdup return values should be checked for NULL to catch the
potential error case of out of memory.
Follow up on
commit 45d50a101e8073191e6d88143990ed91d3bfe815
Ticket: https://ticket.coreboot.org/issues/372
Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M layout.c
1 file changed, 39 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/70006/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/70006
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Gerrit-Change-Number: 70006
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.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-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67479 )
Change subject: spi: Make 'default_spi_write_aai' the default unless defined
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/67479/comment/fb901c4e_6c4057e1
PS1, Line 9: Drop the explicit need to specify the default 'default_spi_write_aai'
> Please mention the struct where you perform the changes on
Done
Patchset:
PS1:
> Two things: […]
Addressed the first part in the commit message by making clear the mathematical constraint here in precise terms.
Regarding the second part. I am not sure renaming everything in the tree with a prefix or suffix of optimised. For one that seems like a different patch to this one that is semantic and secondly that line of reasoning means everything non-NULL is a 'optimised/optional' version which I am not sure makes sense either - 'custom' would be a closer term to the reality but even then makes no sense to use in the function identifier. All we are saying here is to make function pointers in the struct RAII where a default construction of the type into a initial value is valid.
To address the concerns of the second part a alternative compromise would perhaps just be a code comment above spi.c:spi_aai_write().
--
To view, visit https://review.coreboot.org/c/flashrom/+/67479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a
Gerrit-Change-Number: 67479
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Sat, 03 Dec 2022 03:31:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, Peter Marheine,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67479
to look at the new patch set (#4).
Change subject: spi: Make 'default_spi_write_aai' the default unless defined
......................................................................
spi: Make 'default_spi_write_aai' the default unless defined
A NULL func pointer is necessary and sufficient for the
condition `NULL func pointer => default_spi_write_aai' as to not
need this explicit specification of 'default'.
Therefore Drop the explicit need to specify the 'default_spi_write_aai'
callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions.
This simplifies the code and driver development.
Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M bitbang_spi.c
M buspirate_spi.c
M ch341a_spi.c
M digilent_spi.c
M dummyflasher.c
M ft2232_spi.c
M ichspi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M pickit2_spi.c
M raiden_debug_spi.c
M sb600spi.c
M serprog.c
M spi.c
M stlinkv3_spi.c
M usbblaster_spi.c
19 files changed, 23 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/67479/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/67479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a
Gerrit-Change-Number: 67479
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan.
Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, Peter Marheine,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67479
to look at the new patch set (#3).
Change subject: spi: Make 'default_spi_write_aai' the default unless defined
......................................................................
spi: Make 'default_spi_write_aai' the default unless defined
Drop the explicit need to specify the default 'default_spi_write_aai'
callback function pointer in the spi_master struct.
This is a reasonable default for every other driver in the tree
with only a few exceptions.
This simplifies the code and driver development.
Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M bitbang_spi.c
M buspirate_spi.c
M ch341a_spi.c
M digilent_spi.c
M dummyflasher.c
M ft2232_spi.c
M ichspi.c
M jlink_spi.c
M linux_spi.c
M mediatek_i2c_spi.c
M mstarddc_spi.c
M ni845x_spi.c
M pickit2_spi.c
M raiden_debug_spi.c
M sb600spi.c
M serprog.c
M spi.c
M stlinkv3_spi.c
M usbblaster_spi.c
19 files changed, 21 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/67479/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/67479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a
Gerrit-Change-Number: 67479
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Thomas Heijligen, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 )
Change subject: tree/: Drop default_spi_probe_opcode for NULL case
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/70264/comment/a0ac6f64_8f05d5fe
PS1, Line 9: A NULL func pointer is necessary and sufficient for the
> Maybe give a bit more context about what the default function does. Suggestion: […]
It is also necessary as well as sufficient. That phrasing is less exact.
Patchset:
PS1:
> I want to get the erase optimizations from Aarya merged as soon as possible. They touch this also. […]
Absolutely!
--
To view, visit https://review.coreboot.org/c/flashrom/+/70264
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12
Gerrit-Change-Number: 70264
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.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-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 02 Dec 2022 23:12:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan.
Hello Felix Singer, build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/68661
to look at the new patch set (#5).
Change subject: cli_classic.c: Factor out rom bus limits exceeded validation
......................................................................
cli_classic.c: Factor out rom bus limits exceeded validation
Carve out relevant logic relating to validating the rom decode
limits fit within the common buses with certain flash-programmer
pairings into its own function.
Move the comment to the top of the new symbol while we are here
and rewrite the grammar to be a bit more understandable.
Change-Id: Iba1c3c8ab2edc094ede59156ae5e846900fc0777
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M cli_classic.c
1 file changed, 43 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/68661/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/68661
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iba1c3c8ab2edc094ede59156ae5e846900fc0777
Gerrit-Change-Number: 68661
Gerrit-PatchSet: 5
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newpatchset