Attention is currently required from: Daniel Campello, Light.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64668 )
Change subject: cli_output.c: Format progress output to show in single line
......................................................................
Patch Set 1:
(2 comments)
File cli_output.c:
https://review.coreboot.org/c/flashrom/+/64668/comment/307a03bd_a99bab11
PS1, Line 88: msg_ginfo("\r[%s] %u%% complete... ", flashrom_progress_stage_to_string(progress_state->stage), pc);
How does this \r interact with other messages from other sources?
eg:
msg_ginfo("\r[%s] %u%% complete")
msg_gerror("important information")
msg_ginfo("\r[%s] %u%% complete")
https://review.coreboot.org/c/flashrom/+/64668/comment/7c79a67f_9773f694
PS1, Line 89: }
I would assume \r is only appropriate for interactive usage. Is any mechanism already implemented for detecting interactive usage? EG. isatty?
If using isatty, terminal escape sequences may be a better choice (or they might not, I'm not sure). EG: https://chromium-review.googlesource.com/c/external/repo/+/3592792
--
To view, visit https://review.coreboot.org/c/flashrom/+/64668
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If982193f9857f0da47cd71706ce6cebaa60d4323
Gerrit-Change-Number: 64668
Gerrit-PatchSet: 1
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Evan Benn <evanbenn(a)google.com>
Gerrit-CC: Richard Hughes <richard(a)hughsie.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 04:58:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64668 )
Change subject: cli_output.c: Format progress output to show in single line
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Aarya, thank you for the patch! Can I ask you, when you run this, do you see percentage working and increasing from 0 to 100% ? Thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/64668
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If982193f9857f0da47cd71706ce6cebaa60d4323
Gerrit-Change-Number: 64668
Gerrit-PatchSet: 1
Gerrit-Owner: Light <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Richard Hughes <richard(a)hughsie.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 04:33:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nikolai Artemiev.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64851 )
Change subject: libflashrom.map: Add missing functions from libflashrom.h
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/64851/comment/094bc8d5_b16e4484
PS1, Line 7: Add all functions from header
> Suggestion: […]
Done
https://review.coreboot.org/c/flashrom/+/64851/comment/4a3fb9b3_fc0e63da
PS1, Line 9: I added all
: functioned declared in the header.
> Please use imperative. Suggestion: […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/64851
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic90b3c20780d3a07b00bfca82d23d44c4fa6f22f
Gerrit-Change-Number: 64851
Gerrit-PatchSet: 2
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Comment-Date: Wed, 01 Jun 2022 04:26:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev, Evan Benn.
Hello build bot (Jenkins), Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/64851
to look at the new patch set (#2).
Change subject: libflashrom.map: Add missing functions from libflashrom.h
......................................................................
libflashrom.map: Add missing functions from libflashrom.h
Some functions were not being defined, namely flashrom_wp_*. Thus, add
all missing functions from libflashrom.h header file.
Change-Id: Ic90b3c20780d3a07b00bfca82d23d44c4fa6f22f
Signed-off-by: Evan Benn <evanbenn(a)chromium.org>
---
M libflashrom.map
1 file changed, 18 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/64851/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/64851
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic90b3c20780d3a07b00bfca82d23d44c4fa6f22f
Gerrit-Change-Number: 64851
Gerrit-PatchSet: 2
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nikolai Artemiev, Evan Benn.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64851 )
Change subject: libflashrom.map: Add all functions from header
......................................................................
Patch Set 1:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/64851/comment/ac02c284_7b06f337
PS1, Line 7: Add all functions from header
Suggestion:
Add missing functions from libflashrom.h
https://review.coreboot.org/c/flashrom/+/64851/comment/88edb69e_bf75e406
PS1, Line 9: I added all
: functioned declared in the header.
Please use imperative. Suggestion:
Thus, add all missing functions from libflashrom.h header file.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64851
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic90b3c20780d3a07b00bfca82d23d44c4fa6f22f
Gerrit-Change-Number: 64851
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Evan Benn <evanbenn(a)google.com>
Gerrit-Comment-Date: Wed, 01 Jun 2022 04:19:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nikolai Artemiev.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64851 )
Change subject: libflashrom.map: Add all functions from header
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I think its quite plausible that some of these functioned were intentionally omitted. But I just added all of them and we'll see how the review goes.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64851
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic90b3c20780d3a07b00bfca82d23d44c4fa6f22f
Gerrit-Change-Number: 64851
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Comment-Date: Wed, 01 Jun 2022 03:50:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Peter Marheine.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64849 )
Change subject: flashrom_tester: Use Flashrom trait instead of struct FlashromCmd
......................................................................
Patch Set 1:
(1 comment)
File util/flashrom_tester/src/tester.rs:
https://review.coreboot.org/c/flashrom/+/64849/comment/1a9ea5a3_3e22d98e
PS1, Line 55: pub cmd: &'a dyn Flashrom,
The other option suggested by Peter is to take ownership here, with
`pub cmd: Box<dyn Flashrom>`
I had a go at this. I'm not expert in rust enough to make it work; many changes to the WriteProtectState lifetime annotations and 'Sized' trait compilation errors make it tricky.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64849
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie2b4e7e91d69043fd50d1c57f6585fc9946fab10
Gerrit-Change-Number: 64849
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 01 Jun 2022 03:15:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Peter Marheine.
Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64849 )
Change subject: flashrom_tester: Use Flashrom trait instead of struct FlashromCmd
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
There is some further WIP work based on this here: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/…
--
To view, visit https://review.coreboot.org/c/flashrom/+/64849
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie2b4e7e91d69043fd50d1c57f6585fc9946fab10
Gerrit-Change-Number: 64849
Gerrit-PatchSet: 1
Gerrit-Owner: Evan Benn <evanbenn(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 01 Jun 2022 03:11:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment