Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 )
Change subject: tests: Add padding to pci_dev struct for ASAN
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Patchset:
PS4:
I wrapped the lines in commit message so that they fit into 72 chars width limit.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62845
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8
Gerrit-Change-Number: 62845
Gerrit-PatchSet: 4
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 21 Mar 2022 02:05:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 )
Change subject: tests: Add padding to pci_dev struct for ASAN
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Patchset:
PS2:
> the three outstanding patches block the enablement of tests in the cq as each of them are addressing […]
Okay, I see you are not very enthusiastic to fix this comment :) But I am, especially since the situation was already discussed and addressed previously for tests which are using libusb.
I created a patch and put it on the top of this one: CB:62948
I decided that my patch can go on the top of this one for two main reasons:
mock struct pci_dev has been added a while ago, so this patch is modifying existing struct;
and also my patch CB:62948 is introducing dependency for the tests pci/pci.h and I would prefer this to be carefully reviewed to ensure it's all good.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62845
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8
Gerrit-Change-Number: 62845
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 21 Mar 2022 01:51:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62948 )
Change subject: tests: Remove mock struct pci_dev, use real pci symbols in tests
......................................................................
tests: Remove mock struct pci_dev, use real pci symbols in tests
As a follow up on CB:62845 this patch removes mock struct
pci_dev and now tests are including pci/pci.h header and all
real symbols from the header.
Importantly, this means libpci now becomes a dependency
for tests (similar to how libusb is already a dependency for
tests as a result of
commit f47ff316ec79b014a5a37898d46e78e61acd6b01 )
BUG=b:181803212
TEST=ninja test
Change-Id: I1206fbdca392f190066a364376ce0db28071e53c
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M tests/io_mock.h
M tests/tests.c
2 files changed, 3 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/62948/1
diff --git a/tests/io_mock.h b/tests/io_mock.h
index f8f75ad..95bcc6c 100644
--- a/tests/io_mock.h
+++ b/tests/io_mock.h
@@ -35,20 +35,15 @@
#include <stdio.h>
/*
- * Explicitly including the header because some tests are using libusb structs
+ * Explicitly including the headers because some tests are using structs
* in depth, opaque symbols are not sufficient.
*/
#include <libusb.h>
+#include <pci/pci.h>
/* Address value needs fit into uint8_t. */
#define USB_DEVICE_ADDRESS 19
-/* Define struct pci_dev to avoid dependency on pci.h */
-struct pci_dev {
- char padding[18];
- unsigned int device_id;
-};
-
/* POSIX open() flags, avoiding dependency on fcntl.h */
#define O_RDONLY 0
#define O_WRONLY 1
diff --git a/tests/tests.c b/tests/tests.c
index f1fb3f3..111648c 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -48,7 +48,7 @@
}
struct pci_dev mock_pci_dev = {
- .device_id = NON_ZERO,
+ .device_id = (short unsigned int) NON_ZERO,
};
struct pci_dev *__wrap_pcidev_init(void *devs, int bar)
--
To view, visit https://review.coreboot.org/c/flashrom/+/62948
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1206fbdca392f190066a364376ce0db28071e53c
Gerrit-Change-Number: 62948
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 )
Change subject: hwaccess: replace macros by C code
......................................................................
Patch Set 6: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/62898
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a
Gerrit-Change-Number: 62898
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 20 Mar 2022 01:02:48 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment