diff --git a/cli_classic.c b/cli_classic.c index 1a0565bf8..d0c7114f0 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -1282,11 +1282,8 @@ int main(int argc, char *argv[]) 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_v2(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 a8b8d7cf6..42608b3e3 100644 --- a/cli_output.c +++ b/cli_output.c @@ -92,14 +92,13 @@ static void print_progress(const struct cli_progress *cli_progress, enum flashro 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 @@ void flashrom_progress_cb(struct flashrom_flashctx *flashctx) } } - 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/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index 62dffb063..4defe5252 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -51,3 +51,16 @@ requirements are not satisfied or the option is disabled, the authors lists will be replaced with placeholders unless the ``generate_authors_list`` option is set to ``enabled`` in which case the build will fail if the requirements are not satisfied. + +New libflashrom API for progress reporting +------------------------------------------ + +The old ``flashrom_set_progress_callback`` function for requesting progress updates +during library operations is now deprecated. Users should call +``flashrom_set_progress_callback_v2`` instead, which also changes the signature +of the callback function. Specifically, new function type ``flashrom_progress_callback_v2`` +should be used from now on. + +This new API fixes limitations with the old one where most users would need to +define their own global state to track progress, and it was impossible to fix that +issue while maintaining binary compatibility without adding a new API. diff --git a/flashrom.c b/flashrom.c index 8daad747a..dbac12788 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1221,7 +1221,7 @@ notfound: static void setup_progress_from_layout(struct flashctx *flashctx, enum flashrom_progress_stage stage) { - if (!flashctx->progress_callback) + if (!flashctx->progress_callback && !flashctx->deprecated_progress_callback) return; const struct flashrom_layout *const flash_layout = get_layout(flashctx); @@ -1242,7 +1242,7 @@ static void setup_progress_from_layout_and_diff(struct flashctx *flashctx, const void *want, enum flashrom_progress_stage stage) { - if (!flashctx->progress_callback) + if (!flashctx->progress_callback && !flashctx->deprecated_progress_callback) return; const struct flashrom_layout *flash_layout = get_layout(flashctx); diff --git a/include/cli_output.h b/include/cli_output.h index 311c85e72..1a6238257 100644 --- a/include/cli_output.h +++ b/include/cli_output.h @@ -26,6 +26,6 @@ int open_logfile(const char * const filename); 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 80ee5f616..3293980bc 100644 --- a/include/flash.h +++ b/include/flash.h @@ -621,9 +621,12 @@ struct flashrom_flashctx { void *data; } chip_restore_fn[MAX_CHIP_RESTORE_FUNCTIONS]; /* Progress reporting */ - flashrom_progress_callback *progress_callback; - struct flashrom_progress *progress_state; + flashrom_progress_callback_v2 *progress_callback; + struct flashrom_progress progress_state; struct stage_progress stage_progress[FLASHROM_PROGRESS_NR]; + /* deprecated, do not use */ + flashrom_progress_callback *deprecated_progress_callback; + struct flashrom_progress *deprecated_progress_state; /* Maximum allowed % of redundant erase */ int sacrifice_ratio; diff --git a/include/libflashrom.h b/include/libflashrom.h index e87776be1..41c742412 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -75,14 +75,31 @@ enum flashrom_progress_stage { 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); + +/** + * @deprecated Use flashrom_set_progress_callback_v2 instead + */ +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_v2 instead"))); + +typedef void(flashrom_progress_callback_v2)(enum flashrom_progress_stage stage, + size_t current, + size_t total, + void *user_data); + /** * @brief Set the progress callback function. * @@ -90,12 +107,12 @@ typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx); * 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_v2 Pointer to the new progress callback function. + * @param 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_v2(struct flashrom_flashctx *const flashctx, + flashrom_progress_callback_v2 *progress_callback, + void* user_data); /** @} */ /* end flashrom-general */ diff --git a/libflashrom.c b/libflashrom.c index 78e5c9be3..8cc38c463 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -64,15 +64,42 @@ int print(const enum flashrom_log_level level, const char *const fmt, ...) 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(struct flashrom_flashctx *flashctx, + flashrom_progress_callback *progress_callback, + struct flashrom_progress *progress_state) { + msg_gwarn("%s: this function is deprecated (for developers: call %s_v2 instead)\n", + __func__, __func__); + + if (flashctx->progress_callback) { + msg_gwarn("Progress callback already set by %s_v2, " + "ignoring this call since only one progress callback can be registered.\n", + __func__); + return; + } + + flashctx->deprecated_progress_callback = progress_callback; + flashctx->deprecated_progress_state = progress_state; + flashctx->progress_state = *progress_state; +} + +void flashrom_set_progress_callback_v2(struct flashrom_flashctx *flashctx, + flashrom_progress_callback_v2 *progress_callback, + void *user_data) +{ + if (flashctx->deprecated_progress_callback) { + msg_gwarn("Deprecated progress callback already set by flashrom_set_progress_callback, " + "ignoring this call since only one progress callback can be registered.\n"); + return; + } + 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) { - if (flashctx->progress_callback == NULL) + if (!flashctx->progress_callback && !flashctx->deprecated_progress_callback) return; struct stage_progress *stage_progress = &flashctx->stage_progress[stage]; @@ -85,7 +112,7 @@ void init_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_st /** @private */ void update_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_stage stage, size_t increment) { - if (flashctx->progress_callback == NULL) + if (!flashctx->progress_callback && !flashctx->deprecated_progress_callback) return; struct stage_progress *stage_progress = &flashctx->stage_progress[stage]; @@ -96,11 +123,20 @@ void update_progress(struct flashrom_flashctx *flashctx, enum flashrom_progress_ 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); + if (flashctx->progress_callback) { + flashctx->progress_callback(stage, stage_progress->current, stage_progress->total, + flashctx->progress_state.user_data); + } else if (flashctx->deprecated_progress_callback) { + *(flashctx->deprecated_progress_state) = flashctx->progress_state; + flashctx->deprecated_progress_callback(flashctx); + memcpy(&flashctx->progress_state.user_data, + flashctx->deprecated_progress_state->user_data, + sizeof(flashctx->progress_state.user_data)); + } } const char *flashrom_version_info(void) diff --git a/libflashrom.map b/libflashrom.map index e1cdfa6a7..8e4635d84 100644 --- a/libflashrom.map +++ b/libflashrom.map @@ -25,6 +25,7 @@ LIBFLASHROM_1.0 { flashrom_programmer_shutdown; flashrom_set_log_callback; flashrom_set_progress_callback; + flashrom_set_progress_callback_v2; flashrom_shutdown; flashrom_supported_boards; flashrom_supported_chipsets; diff --git a/tests/chip.c b/tests/chip.c index 25bb8db76..95c861655 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -81,20 +81,20 @@ static int block_erase_chip(struct flashctx *flash, unsigned int blockaddr, unsi 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 @@ void erase_chip_with_progress(void **state) 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_v2(&flashctx, progress_callback, &progress_user_data); printf("Erase chip operation started.\n"); assert_int_equal(0, flashrom_flash_erase(&flashctx)); @@ -357,10 +354,7 @@ void read_chip_with_progress(void **state) 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_v2(&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 @@ void write_chip_with_progress(void **state) 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_v2(&flashctx, progress_callback, &progress_user_data); const char *const filename = "-"; unsigned long size = mock_chip.total_size * 1024; @@ -621,10 +612,7 @@ void write_chip_feature_no_erase_with_progress(void **state) 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_v2(&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 a442c7b31..0370312bb 100644 --- a/tests/spi25.c +++ b/tests/spi25.c @@ -69,26 +69,25 @@ int __wrap_spi_send_command(const struct flashctx *flash, 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 @@ void default_spi_read_test_success(void **state) .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_v2(&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 @@ void default_spi_read_test_success(void **state) 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_v2(&flashctx, NULL, NULL); } void spi_write_enable_test_success(void **state)