- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: meson_cross: Introduce meson cross file for DJGPP/DOS
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73439 ) Change subject: meson_cross: Introduce meson cross file for DJGPP/DOS ...................................................................... Patch Set 3: Code-Review+2 (1 comment) File meson_cross/i586_djgpp_dos.txt: https://review.coreboot.org/c/flashrom/+/73439/comment/50b5cbf4_2e3c5a47 PS3, Line 22: gnu99 are gnu extensions actually needed or can one consider just std c99 ? -- To view, visit https://review.coreboot.org/c/flashrom/+/73439 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iabda73942c1e64cd46604c78533982374f78e1a4 Gerrit-Change-Number: 73439 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:32:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson.build: Add 'dos' as option for pci_based programmer
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73156 ) Change subject: meson.build: Add 'dos' as option for pci_based programmer ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73156 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4907a3fd1852ef60d06ef1625ad8a106d89a126 Gerrit-Change-Number: 73156 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:31:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: meson.build: Use library() to build libflashrom
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Thomas Heijligen, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73101 ) Change subject: meson.build: Use library() to build libflashrom ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73101 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27f10fdf1227795a9a3b4e050a2d708b58f10ee7 Gerrit-Change-Number: 73101 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:30:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add build instructions
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Thomas Heijligen, Stefan Reinauer, Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73359 ) Change subject: doc: Add build instructions ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Gerrit-Change-Number: 73359 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:30:28 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Anastasia Klimchuk, Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 5: (17 comments) Patchset: PS5: I added many cleanups you can do as a follow up patch to make type usage consistent, const correct and modern. File ch341a_spi.c: https://review.coreboot.org/c/flashrom/+/72807/comment/664c0111_1089e45e PS3, Line 100: i = 0 `size_t i = 0;` https://review.coreboot.org/c/flashrom/+/72807/comment/27bf8c37_a025c100 PS3, Line 234: unsigned int i; delete https://review.coreboot.org/c/flashrom/+/72807/comment/02bb8f27_97c52287 PS3, Line 235: i probably should be `size_t i = 0;` https://review.coreboot.org/c/flashrom/+/72807/comment/91193b4e_014f5a73 PS3, Line 268: uint8_t `const`? https://review.coreboot.org/c/flashrom/+/72807/comment/5c30bcf0_8f017d2d PS3, Line 311: stored_delay_us `delay_us` https://review.coreboot.org/c/flashrom/+/72807/comment/810fdff2_aed49088 PS3, Line 315: int `unsigned int` ? https://review.coreboot.org/c/flashrom/+/72807/comment/1caaccb1_b84b2414 PS3, Line 323: i = 0 `unsigned int i = 0;` https://review.coreboot.org/c/flashrom/+/72807/comment/178d376f_cd8784cf PS3, Line 365: nsigned int p; delete https://review.coreboot.org/c/flashrom/+/72807/comment/bac944ba_d7931173 PS3, Line 366: p actually should be `size_t p = 0;` https://review.coreboot.org/c/flashrom/+/72807/comment/6aa60df7_80cda1ce PS3, Line 387: i `unsigned int i` https://review.coreboot.org/c/flashrom/+/72807/comment/a46a66f2_64abd509 PS3, Line 419: 4 * 1024 `4*KiB` in another patch? https://review.coreboot.org/c/flashrom/+/72807/comment/c7541d2b_05b52209 PS3, Line 460: ret != 0 `ret`, C will auto-cast to make the predicate boolean. https://review.coreboot.org/c/flashrom/+/72807/comment/9ad093f7_751ad0eb PS3, Line 465: (ret != 0) `(ret)` is canonical. https://review.coreboot.org/c/flashrom/+/72807/comment/4568ca26_e7dcad08 PS3, Line 495: i `int i` in another patch? https://review.coreboot.org/c/flashrom/+/72807/comment/7cec1773_775c1af9 PS3, Line 504: i = 0 ditto https://review.coreboot.org/c/flashrom/+/72807/comment/571daf87_385f8c94 PS3, Line 513: i = 0 ditto -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:29:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Anastasia Klimchuk, Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 5: (1 comment) File ch341a_spi.c: https://review.coreboot.org/c/flashrom/+/72807/comment/012f1d28_321b3ba1 PS3, Line 80: struct libusb_transfer *transfer_out; : struct libusb_transfer *transfer_ins[USB_IN_TRANSFERS]; : : /* Accumulate delays to be plucked between CS deassertion and CS assertions. */ : unsigned int stored_delay_us; : : struct libusb_device_handle *handle; > Done I recommend putting the handle at the start of the struct so that overflows in the above transfer buffers should they happen do not overflow into execution flow memory where function pointers exist. -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:18:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ch341a_spi: drop validation of handle in routines
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72808 ) Change subject: ch341a_spi: drop validation of handle in routines ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72808 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1872495b83a522ceced331fef35d9d9d3b43fce5 Gerrit-Change-Number: 72808 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:16:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Alexander Goncharov. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 5 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 11 Mar 2023 10:16:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: pcidev: remove pcidev_getdevfn() function
by Edward O'Callaghan (Code Review) 11 Mar '23

11 Mar '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73570 ) Change subject: pcidev: remove pcidev_getdevfn() function ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > oh sorry :( what do we do now? PCI access in flashrom is a big mess. Possibly the structure to handle the issues is to have a struct full of pci worker func pointers containing an opaque state machine with the pacc handle inside initialised at `struct bus_cfg` initiation time then passed down into either board_cfg || programmer_cfg or maybe just itself. It is hard to think so many moves ahead as other pre-requirements need to land first like the board_cfg cleanup series. So that could look something akin to; ``` struct flashrom_pcihandle { struct pci_access *pacc; }; struct flashrom_pcibus { struct flashrom_pci handle; int (*find)(..); int (*scan)(..); int (*read)(..); int (*write)(..); }; ``` I don't really like the OOP'ism however the tree has deeply ingrained excessive encapsulation patterns gone rampant and so any procedural state management winds up tripping you up on edge cases. In either case, I would say this commit could be reverted as it seems to be motivated by aesthetic but not fixing a actually issue. The author just didn't notice the `pacc` singleton state machine containment as there was one patch missing that made it truly static. The reason being is that patch got stalled out unfortunately. -- To view, visit https://review.coreboot.org/c/flashrom/+/73570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2983420080f75ae6992edfb032bf5c83b29c803 Gerrit-Change-Number: 73570 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(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, 11 Mar 2023 10:15:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add wrap for __fstat50 to fix tests for NetBSD
by Anastasia Klimchuk (Code Review) 11 Mar '23

11 Mar '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73649 ) Change subject: tests: Add wrap for __fstat50 to fix tests for NetBSD ...................................................................... tests: Add wrap for __fstat50 to fix tests for NetBSD Tested by running unit tests on NetBSD 9.2 Ubuntu 22.04.1 (still pass) Change-Id: Icb8e453328cb40ab9d628f01ecdc3886a233dad5 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c M tests/wraps.h 3 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/73649/1 diff --git a/tests/meson.build b/tests/meson.build index 2de4ee8..df866d7 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -60,6 +60,7 @@ '-Wl,--wrap=__xstat64', '-Wl,--wrap=fstat', '-Wl,--wrap=fstat64', + '-Wl,--wrap=__fstat50', '-Wl,--wrap=__fxstat', '-Wl,--wrap=__fxstat64', '-Wl,--wrap=fileno', diff --git a/tests/tests.c b/tests/tests.c index 911cf47..d296a98 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -229,6 +229,12 @@ return 0; } +int __wrap___fstat50(int fd, void *buf) +{ + LOG_ME; + return 0; +} + int __wrap___fxstat(int fd, void *buf) { LOG_ME; diff --git a/tests/wraps.h b/tests/wraps.h index 6cc9567..089d992 100644 --- a/tests/wraps.h +++ b/tests/wraps.h @@ -46,6 +46,7 @@ int __wrap___xstat64(const char *path, void *buf); int __wrap_fstat(int fd, void *buf); int __wrap_fstat64(int fd, void *buf); +int __wrap___fstat50(int fd, void *buf); int __wrap___fxstat(int fd, void *buf); int __wrap___fxstat64(int fd, void *buf); char *__wrap_fgets(char *buf, int len, FILE *fp); -- To view, visit https://review.coreboot.org/c/flashrom/+/73649 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icb8e453328cb40ab9d628f01ecdc3886a233dad5 Gerrit-Change-Number: 73649 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit