[flashrom] [PATCH] Fix eraseblock walking
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sat Dec 5 01:06:02 CET 2009
On 04.12.2009 17:37, Sean Nelson wrote:
> Looks fine to me! the printf_debug doesn't seem correct, should it be:
> printf_debug("0x%06x-0x%06x, ", start, start+len) ?
> Acked-by: Sean Nelson <audiohacked at gmail.com>
Ouch, yes. Thanks for the review.
Actually, it should be start, start+len-1.
And the code didn't check for broken eraseblock definitions (more than 0
eraseblocks with size 0). We all hope that our eraseblock definitions
are OK, but... Anyway, we need to move some of the checks into a
function that runs on startup and verifies data structure correctness.
That's post 0.9.2 material, though.
New version follows. I'll commit in a few days so interested parties can
find new bugs in my code. ;-)
Fix eraseblock walking and add a few more checks to make sure such bugs
get caught in the future. I found this bug during a code review.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Acked-by: Sean Nelson <audiohacked at gmail.com>
Index: flashrom-blockerase_walk_fix/flashrom.c
===================================================================
--- flashrom-blockerase_walk_fix/flashrom.c (Revision 792)
+++ flashrom-blockerase_walk_fix/flashrom.c (Arbeitskopie)
@@ -773,10 +773,11 @@
int erase_flash(struct flashchip *flash)
{
int i, j, k, ret = 0, found = 0;
+ unsigned int start, len;
printf("Erasing flash chip... ");
for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
- unsigned long done = 0;
+ unsigned int done = 0;
struct block_eraser eraser = flash->block_erasers[k];
printf_debug("Looking at blockwise erase function %i... ", k);
@@ -800,17 +801,39 @@
found = 1;
printf_debug("trying... ");
for (i = 0; i < NUM_ERASEREGIONS; i++) {
+ /* Blocks with zero size are bugs in flashchips.c.
+ * FIXME: This check should be performed on startup.
+ */
+ if (eraser.eraseblocks[i].count &&
+ !eraser.eraseblocks[i].size) {
+ fprintf(stderr, "ERROR: Erase region with size "
+ "0 for this chip. Please report a bug "
+ "at flashrom at flashrom.org\n");
+ ret = 1;
+ break;
+ }
/* count==0 for all automatically initialized array
* members so the loop below won't be executed for them.
*/
for (j = 0; j < eraser.eraseblocks[i].count; j++) {
- ret = eraser.block_erase(flash, done + eraser.eraseblocks[i].size * j, eraser.eraseblocks[i].size);
+ start = done + eraser.eraseblocks[i].size * j;
+ len = eraser.eraseblocks[i].size;
+ printf_debug("0x%06x-0x%06x, ", start,
+ start + len - 1);
+ ret = eraser.block_erase(flash, start, len);
if (ret)
break;
}
if (ret)
break;
+ done += eraser.eraseblocks[i].count *
+ eraser.eraseblocks[i].size;
}
+ printf_debug("\n");
+ if (done != flash->total_size * 1024)
+ fprintf(stderr, "ERROR: Erase region walking erased "
+ "0x%06x bytes total, expected 0x%06x bytes.",
+ done, flash->total_size * 1024);
/* If everything is OK, don't try another erase function. */
if (!ret)
break;
--
Developer quote of the month:
"We are juggling too many chainsaws and flaming arrows and tigers."
More information about the flashrom
mailing list