Change in coreboot[master]: drivers/spi/flashconsole: Fix shadowing local variable

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Mimoja (Code Review)

4 Jan 2020 4 Jan '20

4:05 a.m.

Mimoja has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Commit c9b13594eb8d425e54a126b5c10e3f6fbc41528b removed the g_ prefix from global variables, leaving the local "offset" variable shadowing the global one. This commit partially reverts this by adding a l_ prefix to its two occations.

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 8 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/1

diff --git a/src/drivers/spi/flashconsole.c b/src/drivers/spi/flashconsole.c index 80c63e0..020d282 100644 --- a/src/drivers/spi/flashconsole.c +++ b/src/drivers/spi/flashconsole.c @@ -32,9 +32,9 @@ { uint8_t buffer[READ_BUFFER_SIZE]; size_t size; - size_t offset = 0; size_t len = READ_BUFFER_SIZE; size_t i; + size_t l_offset = 0;

if (fmap_locate_area_as_rdev_rw("CONSOLE", &rdev)) { printk(BIOS_INFO, "Can't find 'CONSOLE' area in FMAP\n"); @@ -60,12 +60,12 @@ return; } if (buffer[i] == 0xff) { - offset += i; + l_offset += i; break; } // If we're done, repeat the process for the next sector if (++i == READ_BUFFER_SIZE) { - offset += len; + l_offset += len; i = 0; } } @@ -75,7 +75,7 @@ return; }

- offset = offset; + offset = l_offset; rdev_ptr = &rdev; }

@@ -96,7 +96,7 @@

void flashconsole_tx_flush(void) { - size_t offset = offset; + size_t l_offset = offset; size_t len = line_offset; size_t region_size; static int busy; @@ -112,17 +112,17 @@

busy = 1; region_size = region_device_sz(rdev_ptr); - if (offset + len >= region_size) + if (l_offset + len >= region_size) len = region_size - offset;

if (rdev_writeat(&rdev, line_buffer, offset, len) != len) return;

// If the region is full, stop future write attempts - if (offset + len >= region_size) + if (l_offset + len >= region_size) return;

- offset = offset + len; + offset = l_offset + len; line_offset = 0;

busy = 0;

Attachments:

attachment.htm (text/html — 5.0 KB)

Show replies by date

HAOUAS Elyes (Code Review)

4 Jan 4 Jan

8:03 a.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 1: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 1 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 07:03:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

9:47 a.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 1:

(7 comments)

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 55: for (i = 0; i < len && offset < size;) { l_offset

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 58: len = MIN(READ_BUFFER_SIZE, size - offset); l_offset

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 59: if (rdev_readat(&rdev, buffer, offset, len) != len) l_offset

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 73: if (offset >= size) { l_offset

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 78: offset = l_offset; This is the only line where s/g_offset/offset happened. That is my argument why this should be the only reference to the global offset.

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 99: size_t l_offset = offset; You could declare this const, so you don't really need the local variable at all.

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 125: offset = l_offset + len; offset += len;

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 1 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 08:47:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Mimoja (Code Review)

1:09 p.m.

Hello Kyösti Mälkki, HAOUAS Elyes, Arthur Heymans, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38171

to look at the new patch set (#2).

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 6 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/2

Mimoja (Code Review)

1:09 p.m.

Hello Kyösti Mälkki, HAOUAS Elyes, Arthur Heymans, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38171

to look at the new patch set (#3).

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 7 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/3

Mimoja (Code Review)

1:10 p.m.

Hello Kyösti Mälkki, HAOUAS Elyes, Arthur Heymans, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38171

to look at the new patch set (#4).

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 7 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/4

Mimoja (Code Review)

1:11 p.m.

Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 4:

(7 comments)

Thank you for activly looking for my tired mistakes. Sorry for the extra work.

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 55: for (i = 0; i < len && offset < size;) {

...

l_offset

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 58: len = MIN(READ_BUFFER_SIZE, size - offset);

...

l_offset

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 59: if (rdev_readat(&rdev, buffer, offset, len) != len)

...

l_offset

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 73: if (offset >= size) {

...

l_offset

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 78: offset = l_offset;

...

This is the only line where s/g_offset/offset happened. […]

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 99: size_t l_offset = offset;

...

You could declare this const, so you don't really need the local variable at all.

Done

https://review.coreboot.org/c/coreboot/+/38171/1/src/drivers/spi/flashconsol... PS1, Line 125: offset = l_offset + len;

...

offset += len;

Done

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 4 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 12:11:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

1:14 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 4: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 4 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 12:14:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

3:18 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 4: Code-Review+1

(2 comments)

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... PS4, Line 35: l_offset I would suggest `tmp_offset` instead

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... PS4, Line 124: offset = offset + len; offset += len;

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 4 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 14:18:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Mimoja (Code Review)

4:06 p.m.

Hello Kyösti Mälkki, Patrick Rudolph, HAOUAS Elyes, Angel Pons, Arthur Heymans, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38171

to look at the new patch set (#5).

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 8 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 5 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Mimoja (Code Review)

4:07 p.m.

Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 5:

(2 comments)

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... File src/drivers/spi/flashconsole.c:

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... PS4, Line 35: l_offset

...

I would suggest `tmp_offset` instead

I went with inital_offset

https://review.coreboot.org/c/coreboot/+/38171/4/src/drivers/spi/flashconsol... PS4, Line 124: offset = offset + len;

...

offset += len;

Ack

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 5 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 15:07:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Mimoja (Code Review)

4:08 p.m.

Hello Kyösti Mälkki, Patrick Rudolph, HAOUAS Elyes, Angel Pons, Arthur Heymans, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38171

to look at the new patch set (#6).

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de --- M src/drivers/spi/flashconsole.c 1 file changed, 8 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/38171/6

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 6 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

HAOUAS Elyes (Code Review)

4:37 p.m.

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 6: Code-Review+1

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 6 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 04 Jan 2020 15:37:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Rudolph (Code Review)

5 Jan 5 Jan

9:42 a.m.

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 6 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 05 Jan 2020 08:42:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

7 Jan 7 Jan

9:49 p.m.

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

Patch Set 6: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 6 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Comment-Date: Tue, 07 Jan 2020 20:49:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki (Code Review)

9:50 p.m.

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38171 )

Change subject: drivers/spi/flashconsole: Fix shadowing local variable ......................................................................

drivers/spi/flashconsole: Fix shadowing local variable

Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Signed-off-by: Johanna Schander coreboot@mimoja.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/38171 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Patrick Rudolph siro@das-labor.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/spi/flashconsole.c 1 file changed, 8 insertions(+), 9 deletions(-)

Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved HAOUAS Elyes: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved

diff --git a/src/drivers/spi/flashconsole.c b/src/drivers/spi/flashconsole.c index 80c63e0..2216e7d 100644 --- a/src/drivers/spi/flashconsole.c +++ b/src/drivers/spi/flashconsole.c @@ -32,7 +32,7 @@ { uint8_t buffer[READ_BUFFER_SIZE]; size_t size; - size_t offset = 0; + size_t initial_offset = 0; size_t len = READ_BUFFER_SIZE; size_t i;

@@ -52,30 +52,30 @@ * the sector is already erased, so we would need to read * anyways to check if it's all 0xff). */ - for (i = 0; i < len && offset < size;) { + for (i = 0; i < len && initial_offset < size;) { // Fill the buffer on first iteration if (i == 0) { len = MIN(READ_BUFFER_SIZE, size - offset); - if (rdev_readat(&rdev, buffer, offset, len) != len) + if (rdev_readat(&rdev, buffer, initial_offset, len) != len) return; } if (buffer[i] == 0xff) { - offset += i; + initial_offset += i; break; } // If we're done, repeat the process for the next sector if (++i == READ_BUFFER_SIZE) { - offset += len; + initial_offset += len; i = 0; } } // Make sure there is still space left on the console - if (offset >= size) { + if (initial_offset >= size) { printk(BIOS_INFO, "No space left on 'console' region in SPI flash\n"); return; }

- offset = offset; + offset = initial_offset; rdev_ptr = &rdev; }

@@ -96,7 +96,6 @@

void flashconsole_tx_flush(void) { - size_t offset = offset; size_t len = line_offset; size_t region_size; static int busy; @@ -122,7 +121,7 @@ if (offset + len >= region_size) return;

- offset = offset + len; + offset += len; line_offset = 0;

busy = 0;

-- To view, visit https://review.coreboot.org/c/coreboot/+/38171 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I246ebdcfd3b973e6a4aa3c15fc5f32497dcf8398 Gerrit-Change-Number: 38171 Gerrit-PatchSet: 7 Gerrit-Owner: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: HAOUAS Elyes ehaouas@noos.fr Gerrit-Reviewer: Kyösti Mälkki kyosti.malkki@gmail.com Gerrit-Reviewer: Mimoja coreboot@mimoja.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Paul Menzel paulepanter@users.sourceforge.net Gerrit-MessageType: merged

1782

days inactive

1785

days old

coreboot-gerrit@coreboot.org

15 comments

5 participants

tags (0)

participants (5)

Angel Pons (Code Review)
HAOUAS Elyes (Code Review)
Kyösti Mälkki (Code Review)
Mimoja (Code Review)
Patrick Rudolph (Code Review)