[flashrom-gerrit] [L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations

[Anastasia Klimchuk (Code Review)]

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk.

Anastasia Klimchuk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email )

Change subject: Complete and fix progress feature implementation for all operations
......................................................................

Patch Set 4:

(4 comments)

Commit Message:

https://review.coreboot.org/c/flashrom/+/84102/comment/978b236f_e60fad00?usp... :
PS1, Line 46: 
            : CLI shares terminal with the rest of the code
> I just meant that flashrom can print log output at any point (like it prints `DONE something`) so CL […]

Thanks, I got it now. I made other comment in the code, on this topic.

File cli_output.c:

https://review.coreboot.org/c/flashrom/+/84102/comment/27f746c1_498ea1c8?usp... :
PS4, Line 119: 16 * FLASHROM_PROGRESS_NR
These would be 48? And then 2 backspaces each time, so 92 backspaces?
The output of one callback is max 5chars stage name + space + 2 digits + % sign + ... and that's 12 chars max?
Why do you need 92 backspaces? Maybe I misunderstand something

Does this relate to the part in commit message:
> The progress doesn't just dump lots of stuff on the screen which

probably won't fly because of CB:64668, but it's not hard to adjust
this.

I read the linked patch, it seems the concern was that if you try to delete previous stage of progress, you can accidentally delete some other messages. But the concern is still valid right?

File include/flash.h:

https://review.coreboot.org/c/flashrom/+/84102/comment/e6eb187b_66cd178c?usp... :
PS1, Line 544: struct stage_progress {
             : 	size_t current;
             : 	size_t total;
             : };
> This is not part of API, nothing in `libflashrom.h` uses this structure.

Acknowledged

https://review.coreboot.org/c/flashrom/+/84102/comment/2ec83f84_35c0d0bd?usp... :
PS1, Line 587: struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
> That would change API and ABI. […]

I think (I think) I understand. From the point of view of libflashrom user, the user enables the progress flag, sets progress callback (maybe with custom user_data), and then just runs whatever operations are needed. So the user controls the callback, but all the information needed for the callback is already contained in flashrom_progress.
Is this correct?