On Thu, 30 Jun 2011 21:34:31 +0200 Stefan Tauner <[email protected]> wrote:
> On Thu, 30 Jun 2011 20:38:33 +0200 > Carl-Daniel Hailfinger <[email protected]> wrote: > > > Am 30.06.2011 20:07 schrieb Carl-Daniel Hailfinger: > > > Am 30.06.2011 19:54 schrieb Carl-Daniel Hailfinger: > > > With the changes outlined above: > > > Acked-by: Carl-Daniel Hailfinger <[email protected]> > > > > > > > Ahem. I should have quoted the part where a new bug appears, and should > > have requested a fix for that. > > > > >>> @@ -1814,13 +1824,13 @@ int chip_safety_check(struct flashchip > > >>> *flash, int force, int read_it, int write } if (erase_it || > > >>> write_it) { /* Write needs erase. */ - if (flash->tested & > > >>> TEST_BAD_ERASE) { + if (flash->tested & TEST_BAD_ERASE && > > > > As discussed on IRC, this should be ||. > > > > > > >>> + !check_block_erasers(flash, 0)) { msg_cerr("Erase is not working > > >>> on this chip. "); if (!force) > > > > And once you switch to || above, you may get a segfault if no block > > erasers exist. Zero usable erase functions is a non-recoverable > > condition, it can not be overridden with --force. > > > > We do perform that check for more than zero usable erase functions > > elsewhere already. Either move that check here (with a separate if > > statement, and add a comment in the original place that checking has > > moved), or leave it where it is and keep the code here as is. > > i dont understand what you are talking about. the erasers are allocated > no matter if they "exist" or not in a fix-sized array. a segfault > could only happen if we try to dereference a block eraser function and > jump there which we don't do here. what access(es) should segfault > exactly? > > i have tried my current, reworked patch with: > "./flashrom -p dummy:bus=spi,emulate="M25P10.RES" -V -c "M25P10.RES" > -E" after deleting the block erasers for that chip i.e. .block_erasers = {}; > i get the expected output of "Erase is not working on this chip. > Aborting." > appended is my current version. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
>From 283d0980f84642784c3f0145987d289eddbf6f7c Mon Sep 17 00:00:00 2001 From: Stefan Tauner <[email protected]> Date: Thu, 30 Jun 2011 21:42:34 +0200 Subject: [PATCH] add count_usable_erasers which returns the number of well-defined erasers for a chip This can be used in various situations (including one in the upcoming SFDP patch) and removes one FIXME in current HEAD. Needed to add a declaration of check_block_eraser. Signed-off-by: Stefan Tauner <[email protected]> --- flashrom.c | 26 +++++++++++++++++++------- 1 files changed, 19 insertions(+), 7 deletions(-) diff --git a/flashrom.c b/flashrom.c index 13d398e..a658032 100644 --- a/flashrom.c +++ b/flashrom.c @@ -465,6 +465,8 @@ struct shutdown_func_data { */ static int may_register_shutdown = 0; +static int check_block_eraser(const struct flashchip *flash, int k, int log); + /* Register a function to be executed on programmer shutdown. * The advantage over atexit() is that you can supply a void pointer which will * be used as parameter to the registered function upon programmer shutdown. @@ -711,6 +713,19 @@ char *extract_programmer_param(const char *param_name) return extract_param(&programmer_param, param_name, ","); } +/* Returns the number of well-defined erasers for a chip. + * The log parameter controls output. */ +static int count_usable_erasers(const struct flashchip *flash, int log) +{ + int usable_erasefunctions = 0; + int k; + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { + if (!check_block_eraser(flash, k, 0)) + usable_erasefunctions++; + } + return usable_erasefunctions; +} + /* start is an offset to the base address of the flash chip */ int check_erased_range(struct flashchip *flash, int start, int len) { @@ -1466,7 +1481,7 @@ static int walk_eraseregions(struct flashchip *flash, int erasefunction, return 0; } -static int check_block_eraser(struct flashchip *flash, int k, int log) +static int check_block_eraser(const struct flashchip *flash, int k, int log) { struct block_eraser eraser = flash->block_erasers[k]; @@ -1495,11 +1510,8 @@ int erase_and_write_flash(struct flashchip *flash, uint8_t *oldcontents, uint8_t int k, ret = 0; uint8_t *curcontents; unsigned long size = flash->total_size * 1024; - int usable_erasefunctions = 0; + int usable_erasefunctions = count_usable_erasers(flash, 0); - for (k = 0; k < NUM_ERASEFUNCTIONS; k++) - if (!check_block_eraser(flash, k, 0)) - usable_erasefunctions++; msg_cinfo("Erasing and writing flash chip... "); if (!usable_erasefunctions) { msg_cerr("ERROR: flashrom has no erase function for this flash " @@ -1818,13 +1830,13 @@ int chip_safety_check(struct flashchip *flash, int force, int read_it, int write } if (erase_it || write_it) { /* Write needs erase. */ - if (flash->tested & TEST_BAD_ERASE) { + if (flash->tested & TEST_BAD_ERASE || + !count_usable_erasers(flash, 0)) { msg_cerr("Erase is not working on this chip. "); if (!force) return 1; msg_cerr("Continuing anyway.\n"); } - /* FIXME: Check if at least one erase function exists. */ } if (write_it) { if (flash->tested & TEST_BAD_WRITE) { -- 1.7.1
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
