Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/86031?usp=email )
Change subject: libflashrom: Update the API for progress callback ......................................................................
libflashrom: Update the API for progress callback
The initial version of API for progress callback would require the callback function to make a second call to get the needed data about progress state (current, total etc).
This patch changes the callback API, so that callback function gets all needed data straight away as parameters, and with this, callback has all the data to do its job.
Since the initial version was submitted and it was in the tree for a while, the change needs to add a _v1 suffix for new thing and deprecated attribute for old thing.
Testing: both unit tests and cli are libflashrom clients. All unit tests run successfully, for the cli all scenarios from commit 75dc0655b95dde91f1426a7e5aecfc04d7b8d631 run successfully.
Change-Id: Ia8cc0461c449b7e65888a64cdc594c55b81eae7a Signed-off-by: Anastasia Klimchuk aklm@flashrom.org --- M cli_classic.c M cli_output.c M include/cli_output.h M include/flash.h M include/libflashrom.h M libflashrom.c M libflashrom.map M tests/chip.c M tests/spi25.c 9 files changed, 66 insertions(+), 68 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/86031/1
diff --git a/cli_classic.c b/cli_classic.c index 1a0565b..d6a3637 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1282,11 +1282,8 @@ fill_flash = &flashes[0];
struct cli_progress cli_progress = {0}; - struct flashrom_progress progress_state = { - .user_data = &cli_progress - }; if (options.show_progress) - flashrom_set_progress_callback(fill_flash, &flashrom_progress_cb, &progress_state); + flashrom_set_progress_callback_v1(fill_flash, &flashrom_progress_cb, &cli_progress);
print_chip_support_status(fill_flash->chip);
diff --git a/cli_output.c b/cli_output.c index a8b8d7c..42608b3 100644 --- a/cli_output.c +++ b/cli_output.c @@ -92,14 +92,13 @@ msg_ginfo("[%s: %2u%%]", flashrom_progress_stage_to_string(stage), cli_progress->stage_pc[stage]); }
-void flashrom_progress_cb(struct flashrom_flashctx *flashctx) +void flashrom_progress_cb(enum flashrom_progress_stage stage, size_t current, size_t total, void* user_data) { - struct flashrom_progress *progress_state = flashctx->progress_state; unsigned int pc = 0; - struct cli_progress *cli_progress = progress_state->user_data; + struct cli_progress *cli_progress = user_data;
/* The expectation is that initial progress of zero is reported before doing anything. */ - if (progress_state->current == 0) { + if (current == 0) { if (!cli_progress->stage_setup) { cli_progress->stage_setup = true;
@@ -113,17 +112,17 @@ } }
- cli_progress->stage_pc[progress_state->stage] = 0; + cli_progress->stage_pc[stage] = 0; } else { cli_progress->stage_setup = false; - cli_progress->visible_stages |= 1 << progress_state->stage; + cli_progress->visible_stages |= 1 << stage; }
- if (progress_state->current > 0 && progress_state->total > 0) - pc = ((unsigned long long) progress_state->current * 100llu) / - ((unsigned long long) progress_state->total); - if (cli_progress->stage_pc[progress_state->stage] != pc) { - cli_progress->stage_pc[progress_state->stage] = pc; + if (current > 0 && total > 0) + pc = ((unsigned long long) current * 100llu) / + ((unsigned long long) total); + if (cli_progress->stage_pc[stage] != pc) { + cli_progress->stage_pc[stage] = pc;
if (line_state == PROGRESS) { /* Erase previous output, because it was previous progress step. */ diff --git a/include/cli_output.h b/include/cli_output.h index 3a3db88..0b916f6 100644 --- a/include/cli_output.h +++ b/include/cli_output.h @@ -26,6 +26,6 @@ int close_logfile(void); void start_logging(void); int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap); -void flashrom_progress_cb(struct flashrom_flashctx *flashctx); +void flashrom_progress_cb(enum flashrom_progress_stage stage, size_t current, size_t total, void* user_data);
#endif /* __CLI_OUTPUT_H__ */ diff --git a/include/flash.h b/include/flash.h index a845186..62d9f42 100644 --- a/include/flash.h +++ b/include/flash.h @@ -621,8 +621,8 @@ void *data; } chip_restore_fn[MAX_CHIP_RESTORE_FUNCTIONS]; /* Progress reporting */ - flashrom_progress_callback *progress_callback; - struct flashrom_progress *progress_state; + flashrom_progress_callback_v1 *progress_callback; + struct flashrom_progress progress_state; struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
/* Maximum allowed % of redundant erase */ diff --git a/include/libflashrom.h b/include/libflashrom.h index e87776b..f1febc9 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -75,14 +75,28 @@ FLASHROM_PROGRESS_ERASE, FLASHROM_PROGRESS_NR, }; + struct flashrom_progress { enum flashrom_progress_stage stage; size_t current; size_t total; void *user_data; }; + struct flashrom_flashctx; typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx); + +void flashrom_set_progress_callback( + struct flashrom_flashctx *const flashctx, + flashrom_progress_callback *progress_callback, + struct flashrom_progress *progress_state) +__attribute__((deprecated("Use flashrom_set_progress_callback_v1 instead"))); + +typedef void(flashrom_progress_callback_v1)(enum flashrom_progress_stage stage, + size_t current, + size_t total, + void* user_data); + /** * @brief Set the progress callback function. * @@ -90,12 +104,12 @@ * to indicate the progress has changed. This allows frontends to do whatever * they see fit with such values, e.g. update a progress bar in a GUI tool. * - * @param progress_callback Pointer to the new progress callback function. - * @param progress_state Pointer to progress state to include with the progress - * callback. + * @param flashrom_progress_callback_v1 Pointer to the new progress callback function. + * @user_data A pointer to a piece of data which is managed by progress caller */ -void flashrom_set_progress_callback(struct flashrom_flashctx *const flashctx, - flashrom_progress_callback *progress_callback, struct flashrom_progress *progress_state); +void flashrom_set_progress_callback_v1(struct flashrom_flashctx *const flashctx, + flashrom_progress_callback_v1 *progress_callback, + void* user_data);
/** @} */ /* end flashrom-general */
diff --git a/libflashrom.c b/libflashrom.c index 78e5c9b..3120636 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -64,10 +64,12 @@ return 0; }
-void flashrom_set_progress_callback(struct flashrom_flashctx *flashctx, flashrom_progress_callback *progress_callback, struct flashrom_progress *progress_state) +void flashrom_set_progress_callback_v1(struct flashrom_flashctx *flashctx, + flashrom_progress_callback_v1 *progress_callback, + void* user_data) { flashctx->progress_callback = progress_callback; - flashctx->progress_state = progress_state; + flashctx->progress_state.user_data = user_data; } /** @private */ void init_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t total) @@ -96,11 +98,12 @@ stage_progress->total = stage_progress->current; }
- flashctx->progress_state->stage = stage; - flashctx->progress_state->current = stage_progress->current; - flashctx->progress_state->total = stage_progress->total; + flashctx->progress_state.stage = stage; + flashctx->progress_state.current = stage_progress->current; + flashctx->progress_state.total = stage_progress->total;
- flashctx->progress_callback(flashctx); + flashctx->progress_callback(stage, stage_progress->current, stage_progress->total, + flashctx->progress_state.user_data); }
const char *flashrom_version_info(void) diff --git a/libflashrom.map b/libflashrom.map index e1cdfa6..cebdd52 100644 --- a/libflashrom.map +++ b/libflashrom.map @@ -25,6 +25,7 @@ flashrom_programmer_shutdown; flashrom_set_log_callback; flashrom_set_progress_callback; + flashrom_set_progress_callback_v1; flashrom_shutdown; flashrom_supported_boards; flashrom_supported_chipsets; diff --git a/tests/chip.c b/tests/chip.c index 25bb8db..1d3b853 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -81,20 +81,20 @@ return 0; }
-static void progress_callback(struct flashctx *flash) { - struct progress_user_data *progress_user_data = flash->progress_state->user_data; - if (flash->progress_state->current == 0) { - printf("Progress started for stage %d, initial callback call\n", flash->progress_state->stage); +static void progress_callback(enum flashrom_progress_stage stage, size_t current, size_t total, void* user_data) { + struct progress_user_data *progress_user_data = user_data; + + if (current == 0) { + printf("Progress started for stage %d, initial callback call\n", stage); } else { /* Progress cannot go backwards. */ - assert_true(flash->progress_state->current >= progress_user_data->last_seen[flash->progress_state->stage]); + assert_true(current >= progress_user_data->last_seen[stage]); }
- if (flash->progress_state->current >= flash->progress_state->total - 1) - printf("Progress almost complete for stage %d, current %ld, total %ld\n", - flash->progress_state->stage, flash->progress_state->current, flash->progress_state->total); + if (current >= total - 1) + printf("Progress almost complete for stage %d, current %ld, total %ld\n", stage, current, total);
- progress_user_data->last_seen[flash->progress_state->stage] = flash->progress_state->current; + progress_user_data->last_seen[stage] = current; }
static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, @@ -255,10 +255,7 @@ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
struct progress_user_data progress_user_data = {0}; - struct flashrom_progress progress_state = { - .user_data = &progress_user_data - }; - flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state); + flashrom_set_progress_callback_v1(&flashctx, progress_callback, &progress_user_data);
printf("Erase chip operation started.\n"); assert_int_equal(0, flashrom_flash_erase(&flashctx)); @@ -357,10 +354,7 @@ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
struct progress_user_data progress_user_data = {0}; - struct flashrom_progress progress_state = { - .user_data = &progress_user_data - }; - flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state); + flashrom_set_progress_callback_v1(&flashctx, progress_callback, &progress_user_data);
const char *const filename = "read_chip.test"; unsigned long size = mock_chip.total_size * 1024; @@ -488,10 +482,7 @@ setup_chip(&flashctx, &layout, &mock_chip, param, &chip_io);
struct progress_user_data progress_user_data = {0}; - struct flashrom_progress progress_state = { - .user_data = &progress_user_data - }; - flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state); + flashrom_set_progress_callback_v1(&flashctx, progress_callback, &progress_user_data);
const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; @@ -621,10 +612,7 @@ assert_non_null(newcontents);
struct progress_user_data progress_user_data = {0}; - struct flashrom_progress progress_state = { - .user_data = &progress_user_data - }; - flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state); + flashrom_set_progress_callback_v1(&flashctx, progress_callback, &progress_user_data);
printf("Write chip operation started.\n"); assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); diff --git a/tests/spi25.c b/tests/spi25.c index a442c7b..ab8ce95 100644 --- a/tests/spi25.c +++ b/tests/spi25.c @@ -69,26 +69,25 @@ return 0; }
-static void spi_read_progress_cb(struct flashrom_flashctx *flashctx) +static void spi_read_progress_cb(enum flashrom_progress_stage stage, size_t current, size_t total, void* user_data) { - struct flashrom_progress *progress_state = flashctx->progress_state; - uint32_t *cnt = (uint32_t *) progress_state->user_data; - assert_int_equal(0x400, progress_state->total); + uint32_t *cnt = (uint32_t *) user_data; + assert_int_equal(0x400, total); switch (*cnt) { case 0: - assert_int_equal(0x0, progress_state->current); + assert_int_equal(0x0, current); break; case 1: - assert_int_equal(0x100, progress_state->current); + assert_int_equal(0x100, current); break; case 2: - assert_int_equal(0x200, progress_state->current); + assert_int_equal(0x200, current); break; case 3: - assert_int_equal(0x300, progress_state->current); + assert_int_equal(0x300, current); break; case 4: - assert_int_equal(0x400, progress_state->current); + assert_int_equal(0x400, current); break; default: fail(); @@ -113,10 +112,7 @@ .chip = &mock_chip, .mst = &mst }; - struct flashrom_progress progress_state = { - .user_data = (void *) &cnt, - }; - flashrom_set_progress_callback(&flashctx, spi_read_progress_cb, &progress_state); + flashrom_set_progress_callback_v1(&flashctx, spi_read_progress_cb, (void *) &cnt); init_progress(&flashctx, FLASHROM_PROGRESS_READ, 0x400); for (int i = 0; i < 4; i++) { will_return(__wrap_spi_send_command, JEDEC_WRDI); @@ -126,7 +122,7 @@ assert_int_equal(0, default_spi_read(&flashctx, buf, offset, sizeof(buf))); assert_int_equal(5, cnt);
- flashrom_set_progress_callback(&flashctx, NULL, NULL); + flashrom_set_progress_callback_v1(&flashctx, NULL, NULL); }
void spi_write_enable_test_success(void **state)