Attention is currently required from: Richard Hughes, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
4 comments:
Patchset:
Any thoughts on the splitting up idea suggested above Richard?
I think the context of adding support to programmers is useful to understand the needs of the framework for reporting progress; splitting this into two changes doesn't seem very useful.
File 82802ab.c:
Patch Set #8, Line 137: set_progress(flash, FLASHROM_PROGRESS_WRITE, i + 1, len);
Updating progress on every byte seems pretty costly. Does it matter? (Are the usual writes to this device small?) Perhaps set_progress itself should include a check to avoid costly too-frequent messages.
It looks like other programmers also update progress on every byte, so it probably should be addressed in set_progress (and apply to library users as well) or flashrom_progress_cb (applying only to flashrom CLI).
File cli_output.c:
Patch Set #8, Line 85: msg_ginfo("[%s] %u%% complete", flashrom_progress_stage_to_string (stage), pc);
Would it make sense to terminate this with a \r to avoid scrolling terminals?
File libflashrom.h:
Patch Set #8, Line 105: typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t current, size_t total, void *user_data);
The progress will go 0-100 for every region being operated on, which might be surprising (but is probably still okay) when operating on multiple flash regions. Might be nice to also report the name of the current region, but I suspect that would be difficult to plumb through.
To view, visit change 49643. To unsubscribe, or for help writing mail filters, visit settings.