Attention is currently required from: Xiang W, Thomas Heijligen, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55115 )
Change subject: programmer_table: remove NULL termination
......................................................................
Patch Set 1: Code-Review+2
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/55115/comment/3f2975e8_27e58026
PS1, Line 7: NULL
lower case `null`, it's not a NULL pointer.
https://review.coreboot.org/c/flashrom/+/55115/comment/e2f0d45a_0a0b3fa0
PS1, Line 9: Object
lower case `object`
https://review.coreboot.org/c/flashrom/+/55115/comment/ec19f1dd_ad1c41c6
PS1, Line 9: correspond
missing singular s: corresponds
https://review.coreboot.org/c/flashrom/+/55115/comment/ff5cffeb_f7224aab
PS1, Line 10: to PROGRAMMER_INVALID has no use in currend code
last but not least, missing full stop
https://review.coreboot.org/c/flashrom/+/55115/comment/1c491a59_f89f475d
PS1, Line 10: currend
current with t
Patchset:
PS1:
Not easy to prove, but I've looked through `git grep -24 programmer_table`:
For most references the index is explicitly guarded. In some cases the
global `programmer` index is used. However, these would dereference
actual NULL pointers inside the programmer struct if `programmer ==
PROGRAMMER_INVALID`. So no trouble there.
--
To view, visit https://review.coreboot.org/c/flashrom/+/55115
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib63c2d2941f23a0788e26e5a5feb25d8669acb42
Gerrit-Change-Number: 55115
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Xiang W <wxjstz(a)126.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 05 Jun 2021 20:18:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55106 )
Change subject: nic3com.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 1:
(2 comments)
File nic3com.c:
https://review.coreboot.org/c/flashrom/+/55106/comment/d74a640c_32cfbd83
PS1, Line 144: msg_perr("Unable to allocate space for PAR master data\n");
Shouldn't we do what nic3com_shutdown() does, here? (or call it
with a locally declared struct nic3com_data instead of the three
individual variables)
https://review.coreboot.org/c/flashrom/+/55106/comment/c65ac3f6_44464d8e
PS1, Line 152: free(data);
Same here? (although this path was already wrong before)
--
To view, visit https://review.coreboot.org/c/flashrom/+/55106
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1c3e4836760cc9f4f9a0bd4294e8d2407b150566
Gerrit-Change-Number: 55106
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-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sat, 05 Jun 2021 16:49:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Stefan Reinauer, Paul Menzel, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54943 )
Change subject: Add AmigaOS suppport to flashrom
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54943/comment/94e8d71f_fb3d73c1
PS1, Line 8:
> How would I do this? Create a Dockerfile.amiga that contains a crosscompile environment?
If there is no way to compile it natively, exactly that. I was lazy with
the DJGPP Dockerfile and just used a published image. Maybe there is
one for AmigaOS too?
If you want to write one, I suggest to base it on a particular Debian
release, so we can get the same results for a few years (the idea of
Manibuilder includes local builds of all Docker images so developers
can investigate on their own without server access). Beside the tool-
chain a user `mani` and a checkout of flashrom in `/home/mani/flashrom`
is expected. And it needs the `mani-wrapper.sh`. I guess you could
copy all that from the DJGPP setup. Cross-compilation is a bit ugly
in the manibuilder/Makefile, but there DJGPP should serve as example
too :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/54943
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3e4a3885f88313aad019454fdbdf4be176b35330
Gerrit-Change-Number: 54943
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sat, 05 Jun 2021 12:50:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Ivan V has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54964 )
Change subject: platform: Fix endianness detection for Apple Silicon Macs
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54964/comment/7de13465_b1643227
PS1, Line 18: 3. __LITTLE_ENDIAN__ and __BIG_ENDIAN__ macros are used only
: for PowerPC architecture.
> This has confused me a little because it describes the flashrom […]
Yes, you're right. I could've worded it better.
--
To view, visit https://review.coreboot.org/c/flashrom/+/54964
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifdb1523ee2c7023e657cfd7b823b091d5deef513
Gerrit-Change-Number: 54964
Gerrit-PatchSet: 2
Gerrit-Owner: Ivan V <root(a)pcm720.me>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Sat, 05 Jun 2021 12:46:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Ivan V.
Hello build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/54964
to look at the new patch set (#2).
Change subject: platform: Fix endianness detection for Apple Silicon Macs
......................................................................
platform: Fix endianness detection for Apple Silicon Macs
Building flashrom on Apple Silicon Macs fails with
"Unable to determine endianness" error. It seems that current
endianness detection fails on macOS due to a combination of
three issues:
1. On macOS, neither GCC nor Clang have __ARMEL__ macros used
by architecture-specific detection;
2. Generic detection fails because Apple uses LITTLE_ENDIAN,
BIG_ENDIAN and BYTE_ORDER macros instead of __BYTE_ORDER and
__LITTLE_ENDIAN;
3. In platform.h, __LITTLE_ENDIAN__ and __BIG_ENDIAN__ macros
are checked only for PowerPC architecture.
This error can be fixed by appending __LITTLE_ENDIAN__ and
__BIG_ENDIAN__ to conditions in IS_ARM branch. I've considered
multiple approaches, but this one seems the cleanest to me.
Signed-off-by: Ivan V <root(a)pcm720.me>
Change-Id: Ifdb1523ee2c7023e657cfd7b823b091d5deef513
---
M platform.h
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/54964/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/54964
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifdb1523ee2c7023e657cfd7b823b091d5deef513
Gerrit-Change-Number: 54964
Gerrit-PatchSet: 2
Gerrit-Owner: Ivan V <root(a)pcm720.me>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ivan V <root(a)pcm720.me>
Gerrit-MessageType: newpatchset
Attention is currently required from: Ivan V.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54964 )
Change subject: platform: Fix endianness detection for Apple Silicon Macs
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54964/comment/b656f61b_afb598da
PS1, Line 18: 3. __LITTLE_ENDIAN__ and __BIG_ENDIAN__ macros are used only
: for PowerPC architecture.
This has confused me a little because it describes the flashrom
code, not what Apple does ^^
Might suffice to change the verb, e.g. "[...] macros are checked [...]".
--
To view, visit https://review.coreboot.org/c/flashrom/+/54964
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifdb1523ee2c7023e657cfd7b823b091d5deef513
Gerrit-Change-Number: 54964
Gerrit-PatchSet: 1
Gerrit-Owner: Ivan V <root(a)pcm720.me>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ivan V <root(a)pcm720.me>
Gerrit-Comment-Date: Sat, 05 Jun 2021 12:34:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment