November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tests: Mock the mode_t variant of open
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69263 ) Change subject: tests: Mock the mode_t variant of open ...................................................................... Patch Set 6: (4 comments) File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/69263/comment/463e3631_3bf22d6c PS4, Line 37: #include <fcntl.h> > Is this a platform-specific header? Is this header present on non-Linux environments? The unit tests […] this one is posix, and posix says: `The <fcntl.h> header shall define the mode_t` Not sure if we need to support non-posix, or what we can do if so. File tests/tests.c: https://review.coreboot.org/c/flashrom/+/69263/comment/2820ca3f_9eef68d8 PS4, Line 21: #include <fcntl.h> > If you need to include the same header twice, this means it needs to be included on a higher level, […] Done https://review.coreboot.org/c/flashrom/+/69263/comment/97060a59_91af6f82 PS4, Line 103: mode_t mode = 0; : if (flags & O_CREAT) { : va_list ap; : va_start(ap, flags); : mode = va_arg(ap, mode_t); : va_end(ap); : } > This code repeats for all variants of open wraps, can this code go to mock_open? It has to be in the varargs function. On the other hand, I don't know the reason for the open64 and open64_2 functions, so maybe they can be removed? File tests/wraps.h: https://review.coreboot.org/c/flashrom/+/69263/comment/3bbdd5d0_042af44e PS4, Line 31: int __wrap_open(const char *pathname, int flags, ...); : int __wrap_open64(const char *pathname, int flags, ...); : int __wrap___open64_2(const char *pathname, int flags, ...); > Just to check, will the wraps still capture two-args variant of `open` calls? yes, at link time there is no information about function type, only the name. This just tells the current translation unit that this is a vararg function. -- To view, visit https://review.coreboot.org/c/flashrom/+/69263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Gerrit-Change-Number: 69263 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 14 Nov 2022 00:23:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add llvm-cov option and run target for code coverage
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Thomas Heijligen, Evan Benn. Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69268 to look at the new patch set (#8). Change subject: tests: Add llvm-cov option and run target for code coverage ...................................................................... tests: Add llvm-cov option and run target for code coverage Code coverage can be requested with -Dllvm_cov and run with ninja llvm-cov. BUG=b:187647884 BRANCH=None TEST=meson test; ninja llvm-cov Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M meson.build M meson_options.txt A scripts/llvm-cov M tests/meson.build 5 files changed, 42 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/69268/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/69268 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Gerrit-Change-Number: 69268 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Add llvm-cov option and run target for code coverage
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Thomas Heijligen, Evan Benn. Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69268 to look at the new patch set (#7). Change subject: tests: Add llvm-cov option and run target for code coverage ...................................................................... tests: Add llvm-cov option and run target for code coverage Code coverage can be requested with -Dllvm_cov and run with ninja llvm-cov. BUG=b:187647884 BRANCH=None TEST=meson test; ninja llvm-cov Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M meson.build M meson_options.txt A scripts/llvm-cov M tests/meson.build 5 files changed, 40 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/69268/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/69268 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Gerrit-Change-Number: 69268 Gerrit-PatchSet: 7 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Detect llvm coverage file io and unmock io functions
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69267 to look at the new patch set (#8). Change subject: tests: Detect llvm coverage file io and unmock io functions ...................................................................... tests: Detect llvm coverage file io and unmock io functions Code coverage writes data to disk, we need to unmock the file io at this point so that the data is really written. BUG=b:187647884 BRANCH=None TEST=llvm-profdata merge -sparse default.profraw -o default.profdata TEST=llvm-cov show ./flashrom_unit_tests -instr-profile=default.profdata --format=html --output-dir=. Change-Id: I21cc1d631e92fa19006b967e85676f108e80b307 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M tests/tests.c M tests/unmock_io.c M tests/wraps.h 4 files changed, 54 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/69267/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/69267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I21cc1d631e92fa19006b967e85676f108e80b307 Gerrit-Change-Number: 69267 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Detect gcov file io and unmock io functions
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Thomas Heijligen. Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69266 to look at the new patch set (#8). Change subject: tests: Detect gcov file io and unmock io functions ...................................................................... tests: Detect gcov file io and unmock io functions Code coverage writes data to disk, we need to unmock the file io at this point so that the data is really written. BUG=b:187647884 BRANCH=None TEST=meson test TEST=ninja coverage Change-Id: If06053ecd78e886c8f7fc55813f4b5635be78c6b Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M Documentation/building.md M tests/meson.build M tests/tests.c A tests/unmock_io.c A tests/unmock_io.h M tests/wraps.h 6 files changed, 124 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/69266/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/69266 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If06053ecd78e886c8f7fc55813f4b5635be78c6b Gerrit-Change-Number: 69266 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Add fwrite and fdopen to io_mock
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Anastasia Klimchuk. Hello Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69538 to look at the new patch set (#2). Change subject: tests: Add fwrite and fdopen to io_mock ...................................................................... tests: Add fwrite and fdopen to io_mock BUG=None BRANCH=None TEST=None Change-Id: I4dff96c264b3ada354538b434b2808fb66c7ef59 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M tests/io_mock.h M tests/tests.c 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/69538/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4dff96c264b3ada354538b434b2808fb66c7ef59 Gerrit-Change-Number: 69538 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Mock the mode_t variant of open
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Angel Pons, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69263 to look at the new patch set (#6). Change subject: tests: Mock the mode_t variant of open ...................................................................... tests: Mock the mode_t variant of open open has a second form with a mode_t argument. When mocking without this argument a caller trying to O_CREAT would have their mode_t argument discarded and a random stack variable would be used instead. BUG=b:187647884 BRANCH=None TEST=meson test Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M tests/io_mock.h M tests/tests.c M tests/tests.h M tests/wraps.h 4 files changed, 53 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/69263/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/69263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Gerrit-Change-Number: 69263 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Move test.h include
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Anastasia Klimchuk. Hello Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69537 to look at the new patch set (#2). Change subject: tests: Move test.h include ...................................................................... tests: Move test.h include BUG=None BRANCH=None TEST=None Change-Id: I8e0611c415c921f5b04b20270fb26e147fefd1b8 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M tests/io_mock.c M tests/io_mock.h 2 files changed, 16 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/69537/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8e0611c415c921f5b04b20270fb26e147fefd1b8 Gerrit-Change-Number: 69537 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: tests: Add fwrite and fdopen to io_mock
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69538 ) Change subject: tests: Add fwrite and fdopen to io_mock ...................................................................... tests: Add fwrite and fdopen to io_mock BUG=None BRANCH=None TEST=None Change-Id: I4dff96c264b3ada354538b434b2808fb66c7ef59 --- M tests/io_mock.h M tests/tests.c 2 files changed, 19 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/69538/1 diff --git a/tests/io_mock.h b/tests/io_mock.h index c9e21ef..3d62ba8 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -107,8 +107,10 @@ FILE* (*iom_fopen)(void *state, const char *pathname, const char *mode); char* (*iom_fgets)(void *state, char *buf, int len, FILE *fp); size_t (*iom_fread)(void *state, void *buf, size_t size, size_t len, FILE *fp); + size_t (*iom_fwrite)(void *state, const void *buf, size_t size, size_t len, FILE *fp); int (*iom_fprintf)(void *state, FILE *fp, const char *fmt, va_list args); int (*iom_fclose)(void *state, FILE *fp); + FILE *(*iom_fdopen)(void *state, int fd, const char *mode); /* * An alternative to custom open mock. A test can either register its diff --git a/tests/tests.c b/tests/tests.c index 04f6182..e9cd62a 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -184,6 +184,8 @@ FILE *__wrap_fdopen(int fd, const char *mode) { LOG_ME; + if (get_io() && get_io()->iom_fdopen) + return get_io()->iom_fdopen(get_io()->state, fd, mode); return not_null(); } @@ -254,6 +256,8 @@ size_t __wrap_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp) { LOG_ME; + if (get_io() && get_io()->iom_fwrite) + return get_io()->iom_fwrite(get_io()->state, ptr, size, nmemb, fp); return nmemb; } -- To view, visit https://review.coreboot.org/c/flashrom/+/69538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4dff96c264b3ada354538b434b2808fb66c7ef59 Gerrit-Change-Number: 69538 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: tests: Move test.h include
by Evan Benn (Code Review) 14 Nov '22

14 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69537 ) Change subject: tests: Move test.h include ...................................................................... tests: Move test.h include BUG=None BRANCH=None TEST=None Change-Id: I8e0611c415c921f5b04b20270fb26e147fefd1b8 --- M tests/io_mock.c M tests/io_mock.h 2 files changed, 15 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/69537/1 diff --git a/tests/io_mock.c b/tests/io_mock.c index c04e9c5..4828e53 100644 --- a/tests/io_mock.c +++ b/tests/io_mock.c @@ -14,7 +14,6 @@ * GNU General Public License for more details. */ -#include <include/test.h> #include "io_mock.h" static const struct io_mock *current_io = NULL; diff --git a/tests/io_mock.h b/tests/io_mock.h index 644da74..a8c84db 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -31,6 +31,8 @@ #ifndef _IO_MOCK_H_ #define _IO_MOCK_H_ +#include <include/test.h> + /* Required for `FILE *` */ #include <stdio.h> -- To view, visit https://review.coreboot.org/c/flashrom/+/69537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8e0611c415c921f5b04b20270fb26e147fefd1b8 Gerrit-Change-Number: 69537 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022