August 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ni845x_spi: Fix signed - unsigned comparisons
by Miklós Márton (Code Review) 17 Aug '21

17 Aug '21

Attention is currently required from: Nico Huber, Miklós Márton, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/56637 to look at the new patch set (#2). Change subject: ni845x_spi: Fix signed - unsigned comparisons ...................................................................... ni845x_spi: Fix signed - unsigned comparisons Change-Id: I48ef927aa28433fb0e3b3a1f3fb2e797095e53bd Signed-off-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M ni845x_spi.c 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/56637/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I48ef927aa28433fb0e3b3a1f3fb2e797095e53bd Gerrit-Change-Number: 56637 Gerrit-PatchSet: 2 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ni845x_spi: handle PROGRAMFILES(X86) env var properly
by Miklós Márton (Code Review) 17 Aug '21

17 Aug '21

Attention is currently required from: Nico Huber, Angel Pons. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56636 ) Change subject: ni845x_spi: handle PROGRAMFILES(X86) env var properly ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: > What I remember from the original review: we used the ${} notation to avoid trouble […] My build VM used during the review was a Windows 7 based one, and now I built a new VM with Windows 10. I have to admit that I have not noted the mingw version used during the review so this might differ from the two setups. Other than these it might be possible that the I simply overlooked the weird -I and -L arguments since it does not harm anything, just looks weird. File Makefile: https://review.coreboot.org/c/flashrom/+/56636/comment/a9cd3e48_75dd899a PS1, Line 750: X > Did the casing change? X instead of x Yes env lists with upper case X. -- To view, visit https://review.coreboot.org/c/flashrom/+/56636 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I397619a5038567d649a417ce6b9d8ac9e1c8c67b Gerrit-Change-Number: 56636 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 17 Aug 2021 20:19:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: opaque_master: Move shutdown function above opaque_master struct
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56824 ) Change subject: opaque_master: Move shutdown function above opaque_master struct ...................................................................... opaque_master: Move shutdown function above opaque_master struct This patch prepares opaque masters to use new API which allows to register shutdown function in opaque_master struct. See also later patch in this chain, where opaque masters are converted to new API. BUG=b:185191942 TEST=builds and ninja test Comparing flashrom binary before and after the patch, make clean && make CONFIG_EVERYTHING=yes VERSION=none binary is the same Change-Id: I5000cfceeba859a76177a17c1cb7d1c1e9fc03fe Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56824 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M linux_mtd.c M nicintel_eeprom.c 2 files changed, 25 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/linux_mtd.c b/linux_mtd.c index 7780feb..42d569d 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -295,6 +295,17 @@ return 0; } +static int linux_mtd_shutdown(void *data) +{ + struct linux_mtd_data *mtd_data = data; + if (mtd_data->dev_fp != NULL) { + fclose(mtd_data->dev_fp); + } + free(data); + + return 0; +} + static const struct opaque_master linux_mtd_opaque_master = { /* max_data_{read,write} don't have any effect for this programmer */ .max_data_read = MAX_DATA_UNSPECIFIED, @@ -356,17 +367,6 @@ return ret; } -static int linux_mtd_shutdown(void *data) -{ - struct linux_mtd_data *mtd_data = data; - if (mtd_data->dev_fp != NULL) { - fclose(mtd_data->dev_fp); - } - free(data); - - return 0; -} - static int linux_mtd_init(void) { char *param; diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index ec0f04e..fd90250 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -395,20 +395,6 @@ return nicintel_ee_write_82580(flash, NULL, addr, len); } -static const struct opaque_master opaque_master_nicintel_ee_82580 = { - .probe = nicintel_ee_probe_82580, - .read = nicintel_ee_read, - .write = nicintel_ee_write_82580, - .erase = nicintel_ee_erase_82580, -}; - -static const struct opaque_master opaque_master_nicintel_ee_i210 = { - .probe = nicintel_ee_probe_i210, - .read = nicintel_ee_read, - .write = nicintel_ee_write_i210, - .erase = nicintel_ee_erase_i210, -}; - static int nicintel_ee_shutdown_i210(void *arg) { if (!done_i20_write) @@ -455,6 +441,20 @@ return ret; } +static const struct opaque_master opaque_master_nicintel_ee_82580 = { + .probe = nicintel_ee_probe_82580, + .read = nicintel_ee_read, + .write = nicintel_ee_write_82580, + .erase = nicintel_ee_erase_82580, +}; + +static const struct opaque_master opaque_master_nicintel_ee_i210 = { + .probe = nicintel_ee_probe_i210, + .read = nicintel_ee_read, + .write = nicintel_ee_write_i210, + .erase = nicintel_ee_erase_i210, +}; + static int nicintel_ee_init(void) { if (rget_io_perms()) -- To view, visit https://review.coreboot.org/c/flashrom/+/56824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5000cfceeba859a76177a17c1cb7d1c1e9fc03fe Gerrit-Change-Number: 56824 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: opaque_master: Add shutdown function in opaque_master struct
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56823 ) Change subject: opaque_master: Add shutdown function in opaque_master struct ...................................................................... opaque_master: Add shutdown function in opaque_master struct With this, register_opaque_master can take care of register_shutdown as well, and every opaque master only needs to call register_opaque_master instead of calling both register_opaque_master and register_shutdown. Next patches in the chain convert opaque masters to use new API. BUG=b:185191942 TEST=builds and ninja test from CB:56413 Change-Id: I34183e6bafc787eec54ee4a26b73a40803f3ce99 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56823 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M opaque.c M programmer.h 2 files changed, 8 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/opaque.c b/opaque.c index f098ffc..7704ec7 100644 --- a/opaque.c +++ b/opaque.c @@ -50,6 +50,13 @@ { struct registered_master rmst = {0}; + if (mst->shutdown) { + if (register_shutdown(mst->shutdown, data)) { + mst->shutdown(data); /* cleanup */ + return 1; + } + } + if (!mst->probe || !mst->read || !mst->write || !mst->erase) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", diff --git a/programmer.h b/programmer.h index ec55987..3de87e5 100644 --- a/programmer.h +++ b/programmer.h @@ -443,6 +443,7 @@ int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); + int (*shutdown)(void *data); void *data; }; int register_opaque_master(const struct opaque_master *mst, void *data); -- To view, visit https://review.coreboot.org/c/flashrom/+/56823 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34183e6bafc787eec54ee4a26b73a40803f3ce99 Gerrit-Change-Number: 56823 Gerrit-PatchSet: 3 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]: linux_mtd: Free param right after its last usage
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56822 ) Change subject: linux_mtd: Free param right after its last usage ...................................................................... linux_mtd: Free param right after its last usage Param is only used in the first half of init function, and it is local, so there is no need to keep it until the end. This makes handling error paths in the second half of init function shorter, because those paths can just return 1 instead of going to a label. BUG=b:185191942 TEST=builds and ninja test from CB:56413 Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56822 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M linux_mtd.c 1 file changed, 6 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/linux_mtd.c b/linux_mtd.c index 284cde4..7780feb 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -404,27 +404,29 @@ msg_pdbg("%s does not exist\n", sysfs_path); goto linux_mtd_init_exit; } + free(param); data = calloc(1, sizeof(*data)); if (!data) { msg_perr("Unable to allocate memory for linux_mtd_data\n"); - goto linux_mtd_init_exit; + return 1; } /* Get MTD info and store it in `data` */ if (linux_mtd_setup(dev_num, data)) { free(data); - goto linux_mtd_init_exit; + return 1; } if (register_shutdown(linux_mtd_shutdown, (void *)data)) { free(data); - goto linux_mtd_init_exit; + return 1; } register_opaque_master(&linux_mtd_opaque_master, data); - ret = 0; + return 0; + linux_mtd_init_exit: free(param); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/56822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9 Gerrit-Change-Number: 56822 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: nicintel_eeprom: Check UNPROG_DEVICE in 82580 shutdown
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56821 ) Change subject: nicintel_eeprom: Check UNPROG_DEVICE in 82580 shutdown ...................................................................... nicintel_eeprom: Check UNPROG_DEVICE in 82580 shutdown Previously shutdown function was registered conditionally for 82580, only if the device was not UNPROG_DEVICE. This patch moves the check for UNPROG_DEVICE into shutdown function itself, so that shutdown function can be always registered for 82580. This also fixes a memory leak in nicintel_ee_shutdown_82580. No changes for i210 device init/shutdown, only for 82580. And very importantly this unlocks API change which plans to move register_shutdown inside register_opaque_master, similar to what's done in CB:56103 BUG=b:185191942 TEST=builds Change-Id: I5c729a3a63d0106e65525a6a77b2f9104c96847f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56821 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M nicintel_eeprom.c 1 file changed, 24 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 430a980..ec0f04e 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -431,18 +431,26 @@ static int nicintel_ee_shutdown_82580(void *eecp) { - uint32_t old_eec = *(uint32_t *)eecp; - /* Request bitbanging and unselect the chip first to be safe. */ - if (nicintel_ee_req() || nicintel_ee_bitset(EEC, EE_CS, 1)) - return -1; + int ret = 0; - /* Try to restore individual bits we care about. */ - int ret = nicintel_ee_bitset(EEC, EE_SCK, old_eec & BIT(EE_SCK)); - ret |= nicintel_ee_bitset(EEC, EE_SI, old_eec & BIT(EE_SI)); - ret |= nicintel_ee_bitset(EEC, EE_CS, old_eec & BIT(EE_CS)); - /* REQ will be cleared by hardware anyway after 2 seconds of inactivity on the SPI pins (3.3.2.1). */ - ret |= nicintel_ee_bitset(EEC, EE_REQ, old_eec & BIT(EE_REQ)); + if (nicintel_pci->device_id != UNPROG_DEVICE) { + uint32_t old_eec = *(uint32_t *)eecp; + /* Request bitbanging and unselect the chip first to be safe. */ + if (nicintel_ee_req() || nicintel_ee_bitset(EEC, EE_CS, 1)) { + ret = -1; + goto out; + } + /* Try to restore individual bits we care about. */ + ret = nicintel_ee_bitset(EEC, EE_SCK, old_eec & BIT(EE_SCK)); + ret |= nicintel_ee_bitset(EEC, EE_SI, old_eec & BIT(EE_SI)); + ret |= nicintel_ee_bitset(EEC, EE_CS, old_eec & BIT(EE_CS)); + /* REQ will be cleared by hardware anyway after 2 seconds of inactivity + * on the SPI pins (3.3.2.1). */ + ret |= nicintel_ee_bitset(EEC, EE_REQ, old_eec & BIT(EE_REQ)); + } + +out: free(eecp); return ret; } @@ -465,6 +473,8 @@ if (!nicintel_eebar) return 1; + uint32_t *eecp = NULL; + nicintel_pci = dev; if (dev->device_id != UNPROG_DEVICE) { uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); @@ -477,15 +487,15 @@ return 1; } - uint32_t *eecp = malloc(sizeof(uint32_t)); + eecp = malloc(sizeof(uint32_t)); if (eecp == NULL) return 1; *eecp = eec; - - if (register_shutdown(nicintel_ee_shutdown_82580, eecp)) - return 1; } + if (register_shutdown(nicintel_ee_shutdown_82580, eecp)) + return 1; + return register_opaque_master(&opaque_master_nicintel_ee_82580, NULL); } else { nicintel_eebar = rphysmap("Intel i210 NIC w/ emulated EEPROM", -- To view, visit https://review.coreboot.org/c/flashrom/+/56821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c729a3a63d0106e65525a6a77b2f9104c96847f Gerrit-Change-Number: 56821 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: nicintel_eeprom: Delete extra parentheses from if condition
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/56820 ) Change subject: nicintel_eeprom: Delete extra parentheses from if condition ...................................................................... nicintel_eeprom: Delete extra parentheses from if condition BUG=185191942 TEST=builds Change-Id: I068bd227668a79f14e38e703a5a49db18d5de8c7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/56820 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M nicintel_eeprom.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index b5f2050..430a980 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -466,7 +466,7 @@ return 1; nicintel_pci = dev; - if ((dev->device_id != UNPROG_DEVICE)) { + if (dev->device_id != UNPROG_DEVICE) { uint32_t eec = pci_mmio_readl(nicintel_eebar + EEC); /* C.f. 3.3.1.5 for the detection mechanism (maybe? contradicting -- To view, visit https://review.coreboot.org/c/flashrom/+/56820 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I068bd227668a79f14e38e703a5a49db18d5de8c7 Gerrit-Change-Number: 56820 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: opaque_master: Use new API to register shutdown function
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56825 ) Change subject: opaque_master: Use new API to register shutdown function ...................................................................... Patch Set 1: Code-Review+1 (2 comments) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/56825/comment/445c183d_d4b32a7b PS1, Line 421: if (register_shutdown(linux_mtd_shutdown, (void *)data)) { Looks like this is also fixing leakage of a file descriptor. File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/56825/comment/254d6472_4efc3b3c PS1, Line 497: : return register_opaque_master(&opaque_master_nicintel_ee_82580, eecp); > The piece of data, eecp, is only used in shutdown function, and not used as opaque_master data (prev […] Sounds correct. -- To view, visit https://review.coreboot.org/c/flashrom/+/56825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id8471a117556edcbf9694752fabe05cf4501ce70 Gerrit-Change-Number: 56825 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 17 Aug 2021 16:28:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ni845x_spi: Fix signed - unsigned comparisons
by Angel Pons (Code Review) 17 Aug '21

17 Aug '21

Attention is currently required from: Nico Huber, Miklós Márton, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56637 ) Change subject: ni845x_spi: Fix signed - unsigned comparisons ...................................................................... Patch Set 1: (1 comment) File ni845x_spi.c: https://review.coreboot.org/c/flashrom/+/56637/comment/81a114b3_d6928366 PS1, Line 556: CS_number = CS_str[0] - '0'; > The operands are first promoted to `int`, so technically this is a conversion […] Interesting, thank you very much! -- To view, visit https://review.coreboot.org/c/flashrom/+/56637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I48ef927aa28433fb0e3b3a1f3fb2e797095e53bd Gerrit-Change-Number: 56637 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 17 Aug 2021 15:25:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ni845x_spi: Fix signed - unsigned comparisons
by Nico Huber (Code Review) 17 Aug '21

17 Aug '21

Attention is currently required from: Miklós Márton, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56637 ) Change subject: ni845x_spi: Fix signed - unsigned comparisons ...................................................................... Patch Set 1: (1 comment) File ni845x_spi.c: https://review.coreboot.org/c/flashrom/+/56637/comment/72c152ff_c72a42b9 PS1, Line 556: CS_number = CS_str[0] - '0'; > Is underflow behavior defined for `unsigned char`? The operands are first promoted to `int`, so technically this is a conversion issue of the negative result. Which is defined as follows: "[...] if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type." With a footnote: "The rules describe arithmetic on the mathematical value, not the value of a given type of expression." If it was a computation on `unsigned` types, this would apply: "A computation involving unsigned operands can never overflow, because a result that cannot be represented by the resulting unsigned integer type is reduced modulo the number that is one greater than the largest value that can be represented by the resulting type." -- To view, visit https://review.coreboot.org/c/flashrom/+/56637 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I48ef927aa28433fb0e3b3a1f3fb2e797095e53bd Gerrit-Change-Number: 56637 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 17 Aug 2021 14:06:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021