October 2021 - flashrom-gerrit

Change in flashrom[master]: tests: Extract libusb wraps into separate file
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/57916 ) Change subject: tests: Extract libusb wraps into separate file ...................................................................... tests: Extract libusb wraps into separate file In preparation for adding more tests with lots of libusb wraps, existing wraps are extracted into separate file. BUG=b:181803212 TEST=builds and ninja test Change-Id: I17c1cc0189fcb742a99ad1b0fd615ab95504a74a Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57916 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- A tests/libusb_wraps.c M tests/meson.build M tests/tests.c 3 files changed, 64 insertions(+), 46 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/tests/libusb_wraps.c b/tests/libusb_wraps.c new file mode 100644 index 0000000..94ef652 --- /dev/null +++ b/tests/libusb_wraps.c @@ -0,0 +1,63 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2021 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <include/test.h> +#include "io_mock.h" + +void *__wrap_usb_dev_get_by_vid_pid_number( + libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) +{ + LOG_ME; + return not_null(); +} + +int __wrap_libusb_set_configuration(libusb_device_handle *devh, int config) +{ + LOG_ME; + return 0; +} + +int __wrap_libusb_claim_interface(libusb_device_handle *devh, int interface_number) +{ + LOG_ME; + return 0; +} + +int __wrap_libusb_control_transfer(libusb_device_handle *devh, uint8_t bmRequestType, + uint8_t bRequest, uint16_t wValue, uint16_t wIndex, unsigned char *data, + uint16_t wLength, unsigned int timeout) +{ + LOG_ME; + if (get_io() && get_io()->libusb_control_transfer) + return get_io()->libusb_control_transfer(get_io()->state, + devh, bmRequestType, bRequest, wValue, wIndex, data, wLength, timeout); + return 0; +} + +int __wrap_libusb_release_interface(libusb_device_handle *devh, int interface_number) +{ + LOG_ME; + return 0; +} + +void __wrap_libusb_close(libusb_device_handle *devh) +{ + LOG_ME; +} + +void __wrap_libusb_exit(libusb_context *ctx) +{ + LOG_ME; +} diff --git a/tests/meson.build b/tests/meson.build index df56358..c55c5d2 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -16,6 +16,7 @@ srcs = [ 'io_mock.c', 'tests.c', + 'libusb_wraps.c', 'helpers.c', 'flashrom.c', 'spi25.c', diff --git a/tests/tests.c b/tests/tests.c index afba5f3..3eb3129 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -282,52 +282,6 @@ return 0; } -void *__wrap_usb_dev_get_by_vid_pid_number( - libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) -{ - LOG_ME; - return not_null(); -} - -int __wrap_libusb_set_configuration(libusb_device_handle *devh, int config) -{ - LOG_ME; - return 0; -} - -int __wrap_libusb_claim_interface(libusb_device_handle *devh, int interface_number) -{ - LOG_ME; - return 0; -} - -int __wrap_libusb_control_transfer(libusb_device_handle *devh, uint8_t bmRequestType, - uint8_t bRequest, uint16_t wValue, uint16_t wIndex, unsigned char *data, - uint16_t wLength, unsigned int timeout) -{ - LOG_ME; - if (get_io() && get_io()->libusb_control_transfer) - return get_io()->libusb_control_transfer(get_io()->state, - devh, bmRequestType, bRequest, wValue, wIndex, data, wLength, timeout); - return 0; -} - -int __wrap_libusb_release_interface(libusb_device_handle *devh, int interface_number) -{ - LOG_ME; - return 0; -} - -void __wrap_libusb_close(libusb_device_handle *devh) -{ - LOG_ME; -} - -void __wrap_libusb_exit(libusb_context *ctx) -{ - LOG_ME; -} - int main(void) { int ret = 0; 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/57916 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17c1cc0189fcb742a99ad1b0fd615ab95504a74a Gerrit-Change-Number: 57916 Gerrit-PatchSet: 6 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Move current_io to io_mock.c be visible across tests
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/57915 ) Change subject: tests: Move current_io to io_mock.c be visible across tests ...................................................................... tests: Move current_io to io_mock.c be visible across tests tests.c is growing and needs to be split, specifically all libusb wraps need to be extracted into their own file. See later in the chain a lot more wraps are added for libusb functions. To be able to split it, current_io needs to be moved one level up, to be visible across tests. This allows having multiple files with wraps, and all the wraps can use current_io. BUG=b:181803212 TEST=builds and ninja test Change-Id: I5327b5de430afe13a8cc931c8b4b188dcb8c8cf6 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57915 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- A tests/io_mock.c M tests/io_mock.h M tests/meson.build M tests/tests.c 4 files changed, 66 insertions(+), 41 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/tests/io_mock.c b/tests/io_mock.c new file mode 100644 index 0000000..51dde92 --- /dev/null +++ b/tests/io_mock.c @@ -0,0 +1,29 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2021, Google Inc. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "io_mock.h" + +static const struct io_mock *current_io = NULL; + +void io_mock_register(const struct io_mock *io) +{ + current_io = io; +} + +const struct io_mock *get_io(void) +{ + return current_io; +} diff --git a/tests/io_mock.h b/tests/io_mock.h index a285e53..7c235d8 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -95,4 +95,6 @@ void io_mock_register(const struct io_mock *io); +const struct io_mock *get_io(void); + #endif diff --git a/tests/meson.build b/tests/meson.build index 8019bd3..df56358 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -14,6 +14,7 @@ root_includes = include_directories('../subprojects') srcs = [ + 'io_mock.c', 'tests.c', 'helpers.c', 'flashrom.c', diff --git a/tests/tests.c b/tests/tests.c index 329e798..afba5f3 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -26,13 +26,6 @@ return (void *)NON_ZERO; } -static const struct io_mock *current_io = NULL; - -void io_mock_register(const struct io_mock *io) -{ - current_io = io; -} - /* Workaround for https://github.com/clibs/cmocka/issues/17 */ char *__wrap_strdup(const char *s) { @@ -84,27 +77,27 @@ int __wrap_open(const char *pathname, int flags) { LOG_ME; - if (current_io && current_io->open) - return current_io->open(current_io->state, pathname, flags); + if (get_io() && get_io()->open) + return get_io()->open(get_io()->state, pathname, flags); return NON_ZERO; } int __wrap_open64(const char *pathname, int flags) { LOG_ME; - if (current_io && current_io->open) - return current_io->open(current_io->state, pathname, flags); + if (get_io() && get_io()->open) + return get_io()->open(get_io()->state, pathname, flags); return NON_ZERO; } int __wrap_ioctl(int fd, unsigned long int request, ...) { LOG_ME; - if (current_io && current_io->ioctl) { + if (get_io() && get_io()->ioctl) { va_list args; int out; va_start(args, request); - out = current_io->ioctl(current_io->state, fd, request, args); + out = get_io()->ioctl(get_io()->state, fd, request, args); va_end(args); return out; } @@ -114,32 +107,32 @@ int __wrap_write(int fd, const void *buf, size_t sz) { LOG_ME; - if (current_io && current_io->write) - return current_io->write(current_io->state, fd, buf, sz); + if (get_io() && get_io()->write) + return get_io()->write(get_io()->state, fd, buf, sz); return sz; } int __wrap_read(int fd, void *buf, size_t sz) { LOG_ME; - if (current_io && current_io->read) - return current_io->read(current_io->state, fd, buf, sz); + if (get_io() && get_io()->read) + return get_io()->read(get_io()->state, fd, buf, sz); return sz; } FILE *__wrap_fopen(const char *pathname, const char *mode) { LOG_ME; - if (current_io && current_io->fopen) - return current_io->fopen(current_io->state, pathname, mode); + if (get_io() && get_io()->fopen) + return get_io()->fopen(get_io()->state, pathname, mode); return not_null(); } FILE *__wrap_fopen64(const char *pathname, const char *mode) { LOG_ME; - if (current_io && current_io->fopen) - return current_io->fopen(current_io->state, pathname, mode); + if (get_io() && get_io()->fopen) + return get_io()->fopen(get_io()->state, pathname, mode); return not_null(); } @@ -170,16 +163,16 @@ char *__wrap_fgets(char *buf, int len, FILE *fp) { LOG_ME; - if (current_io && current_io->fgets) - return current_io->fgets(current_io->state, buf, len, fp); + if (get_io() && get_io()->fgets) + return get_io()->fgets(get_io()->state, buf, len, fp); return NULL; } size_t __wrap_fread(void *ptr, size_t size, size_t len, FILE *fp) { LOG_ME; - if (current_io && current_io->fread) - return current_io->fread(current_io->state, ptr, size, len, fp); + if (get_io() && get_io()->fread) + return get_io()->fread(get_io()->state, ptr, size, len, fp); return 0; } @@ -216,8 +209,8 @@ int __wrap_fclose(FILE *fp) { LOG_ME; - if (current_io && current_io->fclose) - return current_io->fclose(current_io->state, fp); + if (get_io() && get_io()->fclose) + return get_io()->fclose(get_io()->state, fp); return 0; } @@ -247,45 +240,45 @@ void __wrap_test_outb(unsigned char value, unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->outb) - current_io->outb(current_io->state, value, port); + if (get_io() && get_io()->outb) + get_io()->outb(get_io()->state, value, port); } unsigned char __wrap_test_inb(unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->inb) - return current_io->inb(current_io->state, port); + if (get_io() && get_io()->inb) + return get_io()->inb(get_io()->state, port); return 0; } void __wrap_test_outw(unsigned short value, unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->outw) - current_io->outw(current_io->state, value, port); + if (get_io() && get_io()->outw) + get_io()->outw(get_io()->state, value, port); } unsigned short __wrap_test_inw(unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->inw) - return current_io->inw(current_io->state, port); + if (get_io() && get_io()->inw) + return get_io()->inw(get_io()->state, port); return 0; } void __wrap_test_outl(unsigned int value, unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->outl) - current_io->outl(current_io->state, value, port); + if (get_io() && get_io()->outl) + get_io()->outl(get_io()->state, value, port); } unsigned int __wrap_test_inl(unsigned short port) { /* LOG_ME; */ - if (current_io && current_io->inl) - return current_io->inl(current_io->state, port); + if (get_io() && get_io()->inl) + return get_io()->inl(get_io()->state, port); return 0; } @@ -313,8 +306,8 @@ uint16_t wLength, unsigned int timeout) { LOG_ME; - if (current_io && current_io->libusb_control_transfer) - return current_io->libusb_control_transfer(current_io->state, + if (get_io() && get_io()->libusb_control_transfer) + return get_io()->libusb_control_transfer(get_io()->state, devh, bmRequestType, bRequest, wValue, wIndex, data, wLength, timeout); return 0; } 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/57915 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5327b5de430afe13a8cc931c8b4b188dcb8c8cf6 Gerrit-Change-Number: 57915 Gerrit-PatchSet: 6 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Move LOG_ME to include/tests.h to be available everywhere
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/57914 ) Change subject: tests: Move LOG_ME to include/tests.h to be available everywhere ...................................................................... tests: Move LOG_ME to include/tests.h to be available everywhere LOG_ME macro is very generic and can be useful anywhere in tests. Previously was only used in scope of tests.c. With time more tests are added, and more files, LOG_ME needs to be visible everywhere. BUG=b:181803212 TEST=builds and ninja test Change-Id: If7f3d256161bc8b81e996328e445cccab9a82174 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57914 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/include/test.h M tests/tests.c 2 files changed, 2 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/tests/include/test.h b/tests/include/test.h index ee7b4cf..bf8c4f7 100644 --- a/tests/include/test.h +++ b/tests/include/test.h @@ -35,4 +35,6 @@ */ void *not_null(void); +#define LOG_ME printf("%s is called\n", __func__) + #endif /* _TESTS_TEST_H */ diff --git a/tests/tests.c b/tests/tests.c index 9dadddf..329e798 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -21,9 +21,6 @@ #include <string.h> #include <stdint.h> -/* redefinitions/wrapping */ -#define LOG_ME printf("%s is called\n", __func__) - void *not_null(void) { return (void *)NON_ZERO; 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/57914 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7f3d256161bc8b81e996328e445cccab9a82174 Gerrit-Change-Number: 57914 Gerrit-PatchSet: 6 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add NON_ZERO macro and not_null function instead of MOCK_HANDLE
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/57269 ) Change subject: tests: Add NON_ZERO macro and not_null function instead of MOCK_HANDLE ...................................................................... tests: Add NON_ZERO macro and not_null function instead of MOCK_HANDLE This patch adds NON_ZERO macro and not_null function into io_mock.h, so that they can be used anywhere in tests. Common usage for not_null is to indicate a valid pointer, where it doesn't matter what the pointer is, only matters it is not null. Common usage of NON_ZERO is to indicate a valid file descriptor, where it only matters the descriptor is non-zero integer. New features replace all usages of previous MOCK_HANDLE. This patch corrects return value from __wrap_ioctl to be successful by default. It used to be MOCK_HANDLE, but should be 0. Included in this patch because this is also a replacement of MOCK_HANDLE. BUG=b:181803212 TEST=builds and ninja test Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/57269 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/include/test.h M tests/init_shutdown.c M tests/tests.c 3 files changed, 23 insertions(+), 13 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/tests/include/test.h b/tests/include/test.h index 24fa963..ee7b4cf 100644 --- a/tests/include/test.h +++ b/tests/include/test.h @@ -27,4 +27,12 @@ #include <setjmp.h> #include <cmocka.h> +#define NON_ZERO (0xf000baaa) + +/* + * Having this as function allows to set a breakpoint on the address, + * as it has a named symbol associated with the address number. + */ +void *not_null(void); + #endif /* _TESTS_TEST_H */ diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 90bde1e..03e888f 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -19,8 +19,6 @@ #include "io_mock.h" #include "programmer.h" -#define NOT_NULL ((void *)0xf000baaa) - static void run_lifecycle(void **state, const struct programmer_entry *prog, const char *param) { (void) state; /* unused */ @@ -100,7 +98,7 @@ io_state->fopen_path = strdup(pathname); - return NOT_NULL; + return not_null(); } size_t linux_mtd_fread(void *state, void *buf, size_t size, size_t len, FILE *fp) diff --git a/tests/tests.c b/tests/tests.c index ac59470..9dadddf 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -23,7 +23,11 @@ /* redefinitions/wrapping */ #define LOG_ME printf("%s is called\n", __func__) -#define MOCK_HANDLE 2021 + +void *not_null(void) +{ + return (void *)NON_ZERO; +} static const struct io_mock *current_io = NULL; @@ -54,7 +58,7 @@ } struct pci_dev mock_pci_dev = { - .device_id = MOCK_HANDLE, + .device_id = NON_ZERO, }; struct pci_dev *__wrap_pcidev_init(void *devs, int bar) @@ -66,7 +70,7 @@ uintptr_t __wrap_pcidev_readbar(void *dev, int bar) { LOG_ME; - return MOCK_HANDLE; + return NON_ZERO; } void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) @@ -85,7 +89,7 @@ LOG_ME; if (current_io && current_io->open) return current_io->open(current_io->state, pathname, flags); - return MOCK_HANDLE; + return NON_ZERO; } int __wrap_open64(const char *pathname, int flags) @@ -93,7 +97,7 @@ LOG_ME; if (current_io && current_io->open) return current_io->open(current_io->state, pathname, flags); - return MOCK_HANDLE; + return NON_ZERO; } int __wrap_ioctl(int fd, unsigned long int request, ...) @@ -107,7 +111,7 @@ va_end(args); return out; } - return MOCK_HANDLE; + return 0; } int __wrap_write(int fd, const void *buf, size_t sz) @@ -131,7 +135,7 @@ LOG_ME; if (current_io && current_io->fopen) return current_io->fopen(current_io->state, pathname, mode); - return (void *)MOCK_HANDLE; + return not_null(); } FILE *__wrap_fopen64(const char *pathname, const char *mode) @@ -139,7 +143,7 @@ LOG_ME; if (current_io && current_io->fopen) return current_io->fopen(current_io->state, pathname, mode); - return (void *)MOCK_HANDLE; + return not_null(); } int __wrap_stat(const char *path, void *buf) @@ -197,7 +201,7 @@ int __wrap_fileno(FILE *fp) { LOG_ME; - return MOCK_HANDLE; + return NON_ZERO; } int __wrap_fsync(int fd) @@ -292,7 +296,7 @@ libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) { LOG_ME; - return (void *)MOCK_HANDLE; + return not_null(); } int __wrap_libusb_set_configuration(libusb_device_handle *devh, int config) 6 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/57269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Gerrit-Change-Number: 57269 Gerrit-PatchSet: 8 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add wraps for all variants of stat
by Nico Huber (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58103 ) Change subject: tests: Add wraps for all variants of stat ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/58103/comment/683a4b09_b4c4a0d6 PS2, Line 9: all When considering non-standard symbol names, there is no "all". People could name it whatever they want. Please mention what environment this exactly applies to. If we don't keep track of such things, we'd have to carry the baggage forever (we could never be sure what breaks if some quirk handling would be removed). -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 Gerrit-PatchSet: 2 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: Mon, 18 Oct 2021 09:49:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add wraps for all variants of stat
by Edward O'Callaghan (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58103 ) Change subject: tests: Add wraps for all variants of stat ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Oct 2021 04:02:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add wraps for all variants of stat
by Edward O'Callaghan (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58103 ) Change subject: tests: Add wraps for all variants of stat ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/58103/comment/d1bfaa58_5e6aefe6 PS2, Line 17: TEST=running tests on two different environments, : with 1) stat64 and 2) __xstat64 invoked this does sound like minijail sort of thing although the change is harmless. I would investigate if this was a policy change in the test environment before being sure we need this wrap? -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 Gerrit-PatchSet: 2 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 18 Oct 2021 03:58:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add init-shutdown test for raiden_debug_spi
by Anastasia Klimchuk (Code Review) 18 Oct '21

18 Oct '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57918 ) Change subject: tests: Add init-shutdown test for raiden_debug_spi ...................................................................... Patch Set 5: (2 comments) This change is ready for review. File tests/init_shutdown.c: https://review.coreboot.org/c/flashrom/+/57918/comment/78f4786d_74f2cf50 PS5, Line 122: (void *) I added cast to avoid warnings about discarding const qualifier, hope it's ok. File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/57918/comment/16a3c10b_09495b96 PS4, Line 42: struct libusb_endpoint_descriptor { : uint8_t offset1[2]; : uint8_t bEndpointAddress; : uint8_t bmAttributes; : uint8_t offset2[5]; : unsigned char *extra; : int extra_length; : }; : typedef struct libusb_endpoint_descriptor libusb_endpoint_descriptor; : : struct libusb_interface_descriptor { : uint8_t offset1[2]; : uint8_t bInterfaceNumber; : uint8_t bAlternateSetting; : uint8_t bNumEndpoints; : uint8_t bInterfaceClass; : uint8_t bInterfaceSubClass; : uint8_t bInterfaceProtocol; : uint8_t offset2; : struct libusb_endpoint_descriptor *endpoint; : }; : typedef struct libusb_interface_descriptor libusb_interface_descriptor; : : struct libusb_interface { : struct libusb_interface_descriptor *altsetting; : int num_altsetting; : }; : typedef struct libusb_interface libusb_interface; : : struct libusb_config_descriptor { : uint8_t offset1[4]; : uint8_t bNumInterfaces; : uint8_t bConfigurationValue; : uint8_t offset2[3]; : struct libusb_interface *interface; : }; : typedef struct libusb_config_descriptor libusb_config_descriptor; : : struct libusb_device {}; : typedef struct libusb_device libusb_device; : : struct libusb_device_descriptor { : uint8_t offset1[8]; : uint16_t idVendor; : uint16_t idProduct; : uint8_t offset2[5]; : uint8_t bNumConfigurations; : }; : typedef struct libusb_device_descriptor libusb_device_descriptor; > > I will re-work this patch, so that tests just include libusb.h always. […] I included the header only once in io_mock.h and it seems to work! One thing I am not sure about, you said to "require libusb to be around to build the tests", does it mean require it in meson build? I haven't done this last bit yet, need to see what is the right way to express the requirement. As an experiment, I changed the line into #include <libusb666.h> and the error is: ../tests/io_mock.h:41:10: fatal error: libusb666.h: No such file or directory maybe it's not a user-friendly error... -- To view, visit https://review.coreboot.org/c/flashrom/+/57918 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I880a8637ab02de179df9169c1898230bce4dc1c7 Gerrit-Change-Number: 57918 Gerrit-PatchSet: 5 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: Thomas Heijligen <src(a)posteo.de> 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-Comment-Date: Mon, 18 Oct 2021 02:55:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic.c: Make -r/-w/-v argument optional
by Anastasia Klimchuk (Code Review) 17 Oct '21

17 Oct '21

Attention is currently required from: Nico Huber, David Hendricks, Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52362 ) Change subject: cli_classic.c: Make -r/-w/-v argument optional ...................................................................... Patch Set 18: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/52362/comment/9ec47735_85bb460b PS18, Line 733: for (arg = include_args; arg; arg = arg->next) { : if (check_filename(arg->file, "region")) { : ret = 1; : goto out_shutdown; : } : } Not sure what is the status of this patch, but seems like it needs to be updated because I have just modified this chunk of code in chromium tree: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/… (hope the link is visible to everyone? if not tell me please). I read the latest few comments, and it not entirely clear to me what is the plan for this patch: complete or abandon? -- To view, visit https://review.coreboot.org/c/flashrom/+/52362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41 Gerrit-Change-Number: 52362 Gerrit-PatchSet: 18 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> 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: Sun, 17 Oct 2021 22:54:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: replace flashrom specific macros by compiler defines
by Nico Huber (Code Review) 15 Oct '21

15 Oct '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58280 ) Change subject: hwaccess: replace flashrom specific macros by compiler defines ...................................................................... hwaccess: replace flashrom specific macros by compiler defines Replace the remaining IS_* macros with the associated compiler defines Change-Id: Ia0f022d12390722816066d292e1878824adc613c Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58280 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M dediprog.c M developerbox_spi.c M dmi.c M flash.h M hwaccess.c M hwaccess.h M internal.c M pickit2_spi.c D platform.h M serial.c M serprog.c 11 files changed, 20 insertions(+), 113 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/dediprog.c b/dediprog.c index 85b78c8..939673c 100644 --- a/dediprog.c +++ b/dediprog.c @@ -15,8 +15,6 @@ * GNU General Public License for more details. */ -#include "platform.h" - #include <sys/types.h> #include <stdlib.h> #include <stdio.h> diff --git a/developerbox_spi.c b/developerbox_spi.c index 5573d6f..8c4a8df 100644 --- a/developerbox_spi.c +++ b/developerbox_spi.c @@ -31,8 +31,6 @@ * should be turned on). */ -#include "platform.h" - #include <stdlib.h> #include <libusb.h> #include "programmer.h" diff --git a/dmi.c b/dmi.c index e449cd9..5d709f1 100644 --- a/dmi.c +++ b/dmi.c @@ -30,7 +30,6 @@ #include <stdio.h> #include <stdlib.h> -#include "platform.h" #include "flash.h" #include "programmer.h" diff --git a/flash.h b/flash.h index 3baa076..391a2d4 100644 --- a/flash.h +++ b/flash.h @@ -20,8 +20,6 @@ #ifndef __FLASH_H__ #define __FLASH_H__ 1 -#include "platform.h" - #include <inttypes.h> #include <stdio.h> #include <stdint.h> diff --git a/hwaccess.c b/hwaccess.c index 13bacd7..9bfd8ea 100644 --- a/hwaccess.c +++ b/hwaccess.c @@ -14,8 +14,6 @@ * GNU General Public License for more details. */ -#include "platform.h" - #include <stdint.h> #include <string.h> #include <stdlib.h> @@ -34,7 +32,7 @@ #include <sys/io.h> #endif -#if IS_X86 && USE_DEV_IO +#if (defined (__i386__) || defined (__x86_64__) || defined(__amd64__)) && USE_DEV_IO int io_fd; #endif @@ -50,9 +48,12 @@ * * See also https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Docume… */ -#if IS_PPC // cf. http://lxr.free-electrons.com/source/arch/powerpc/include/asm/barrier.h +// cf. http://lxr.free-electrons.com/source/arch/powerpc/include/asm/barrier.h +#if defined(__powerpc) || defined(__powerpc__) || defined(__powerpc64__) || defined(__POWERPC__) || \ + defined(__ppc__) || defined(__ppc64__) || defined(_M_PPC) || defined(_ARCH_PPC) || \ + defined(_ARCH_PPC64) || defined(__ppc) asm("eieio" : : : "memory"); -#elif IS_SPARC +#elif (__sparc__) || defined (__sparc) #if defined(__sparc_v9__) || defined(__sparcv9) /* Sparc V9 CPUs support three different memory orderings that range from x86-like TSO to PowerPC-like * RMO. The modes can be switched at runtime thus to make sure we maintain the right order of access we @@ -69,7 +70,7 @@ #endif } -#if IS_X86 && !(defined(__DJGPP__) || defined(__LIBPAYLOAD__)) +#if (defined (__i386__) || defined (__x86_64__) || defined(__amd64__)) && !(defined(__DJGPP__) || defined(__LIBPAYLOAD__)) static int release_io_perms(void *p) { #if defined (__sun) @@ -83,13 +84,11 @@ #endif return 0; } -#endif /* Get I/O permissions with automatic permission release on shutdown. */ int rget_io_perms(void) { -#if IS_X86 && !(defined(__DJGPP__) || defined(__LIBPAYLOAD__)) -#if defined (__sun) + #if defined (__sun) if (sysi86(SI86V86, V86SC_IOPL, PS_IOPL) != 0) { #elif USE_DEV_IO if ((io_fd = open("/dev/io", O_RDWR)) < 0) { @@ -111,13 +110,19 @@ } else { register_shutdown(release_io_perms, NULL); } -#else - /* DJGPP and libpayload environments have full PCI port I/O permissions by default. */ - /* PCI port I/O support is unimplemented on PPC/MIPS and unavailable on ARM. */ -#endif return 0; } +#else + +/* DJGPP and libpayload environments have full PCI port I/O permissions by default. */ +/* PCI port I/O support is unimplemented on PPC/MIPS and unavailable on ARM. */ +int rget_io_perms(void) +{ + return 0; +} +#endif + void mmio_writeb(uint8_t val, void *addr) { *(volatile uint8_t *) addr = val; diff --git a/hwaccess.h b/hwaccess.h index 5097ac9..56c2d1e 100644 --- a/hwaccess.h +++ b/hwaccess.h @@ -20,8 +20,6 @@ #ifndef __HWACCESS_H__ #define __HWACCESS_H__ 1 -#include "platform.h" - #if NEED_PCI == 1 /* * libpci headers use the variable name "index" which triggers shadowing @@ -109,8 +107,7 @@ #define le_to_cpu32 cpu_to_le32 #define le_to_cpu64 cpu_to_le64 -#if NEED_RAW_ACCESS == 1 -#if IS_X86 +#if NEED_RAW_ACCESS == 1 && (defined (__i386__) || defined (__x86_64__) || defined(__amd64__)) #include "hwaccess_x86_io.h" @@ -140,31 +137,7 @@ #define wrmsr libpayload_wrmsr #endif -#elif IS_PPC -/* PCI port I/O is not yet implemented on PowerPC. */ - -#elif IS_MIPS - -/* PCI port I/O is not yet implemented on MIPS. */ - -#elif IS_SPARC - -/* PCI port I/O is not yet implemented on SPARC. */ - -#elif IS_ARM - -/* Non memory mapped I/O is not supported on ARM. */ - -#elif IS_ARC - -/* Non memory mapped I/O is not supported on ARC. */ - -#else - -#error Unknown architecture, please check if it supports PCI port IO. - -#endif /* IS_* */ -#endif /* NEED_RAW_ACCESS == 1 */ +#endif #endif /* !__HWACCESS_H__ */ diff --git a/internal.c b/internal.c index b9d17b4..43bed9e 100644 --- a/internal.c +++ b/internal.c @@ -18,7 +18,6 @@ #include <string.h> #include <stdlib.h> #include "flash.h" -#include "platform.h" #include "programmer.h" #include "hwaccess.h" diff --git a/pickit2_spi.c b/pickit2_spi.c index 17da1d9..c090efe 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -32,8 +32,6 @@ * PICkit2 code: https://github.com/steve-m/avrdude/blob/master/pickit2.c */ -#include "platform.h" - #include <stdlib.h> #include <stdio.h> #include <string.h> diff --git a/platform.h b/platform.h deleted file mode 100644 index e0f8a73..0000000 --- a/platform.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * This file is part of the flashrom project. - * - * Copyright (C) 2011 Carl-Daniel Hailfinger - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -/* - * Header file to determine target OS and CPU architecture. - */ - -#ifndef __PLATFORM_H__ -#define __PLATFORM_H__ 1 - -// Likewise for target architectures -#if defined (__i386__) || defined (__x86_64__) || defined(__amd64__) - #define IS_X86 1 -#elif defined (__mips) || defined (__mips__) || defined (__MIPS__) || defined (mips) - #define IS_MIPS 1 -#elif defined(__powerpc) || defined(__powerpc__) || defined(__powerpc64__) || defined(__POWERPC__) || \ - defined(__ppc__) || defined(__ppc64__) || defined(_M_PPC) || defined(_ARCH_PPC) || \ - defined(_ARCH_PPC64) || defined(__ppc) - #define IS_PPC 1 -#elif defined(__arm__) || defined(__TARGET_ARCH_ARM) || defined(_ARM) || defined(_M_ARM) || defined(__arm) || \ - defined(__aarch64__) - #define IS_ARM 1 -#elif defined (__sparc__) || defined (__sparc) - #define IS_SPARC 1 -#elif defined (__alpha__) - #define IS_ALPHA 1 -#elif defined (__hppa__) || defined (__hppa) - #define IS_HPPA 1 -#elif defined (__m68k__) - #define IS_M68K 1 -#elif defined (__riscv) - #define IS_RISCV 1 -#elif defined (__sh__) - #define IS_SH 1 -#elif defined(__s390__) || defined(__s390x__) || defined(__zarch__) - #define IS_S390 1 -#elif defined(__arc__) - #define IS_ARC 1 -#endif - -#if !(IS_X86 || IS_MIPS || IS_PPC || IS_ARM || IS_SPARC || IS_ALPHA || IS_HPPA || IS_M68K || IS_RISCV || IS_SH || IS_S390 || IS_ARC) -#error Unknown architecture -#endif - -#endif /* !__PLATFORM_H__ */ diff --git a/serial.c b/serial.c index 76d34a2..72f9ef6 100644 --- a/serial.c +++ b/serial.c @@ -15,8 +15,6 @@ * GNU General Public License for more details. */ -#include "platform.h" - #include <stdio.h> #include <stdlib.h> #include <unistd.h> diff --git a/serprog.c b/serprog.c index f142b49..2bbf44b 100644 --- a/serprog.c +++ b/serprog.c @@ -15,8 +15,6 @@ * GNU General Public License for more details. */ -#include "platform.h" - #include <stdio.h> #if ! IS_WINDOWS /* stuff (presumably) needed for sockets only */ #include <stdlib.h> 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/58280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia0f022d12390722816066d292e1878824adc613c Gerrit-Change-Number: 58280 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2021