On Fri, 01 Jul 2011 00:19:30 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 30.06.2011 21:34 schrieb Stefan Tauner:
On Thu, 30 Jun 2011 20:38:33 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net 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 c-d.hailfinger.devel.2006@gmx.net
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."
Did you also try specifying --force as well? That should get further along until it tries to actually write/erase. We have precautions there against empty erasers, so the segfault would not happen unless you axe the now redundant check for usable_erasefunctions!=0 in erase_and_write_flash(). Mh. There are even more safety checks in that code, so we might be fine. I need to recheck this.
jup: Erase is not working on this chip. Continuing anyway. Erasing and writing flash chip... ERROR: flashrom has no erase function for this flash chip. Your flash chip is in an unknown state.
that is the "redundant" check you are talking about in action afaiks. removing that results in: Erase is not working on this chip. Continuing anyway. Erasing and writing flash chip... Looking at blockwise erase function 0... not defined. trying...
Done. that is because the body of the inner loop in walk_eraseregions is never executed because eraser.eraseblocks[i].count is always 0 hence the NULL pointer is never dereferenced.
the check for usable_erasefunctions!=0 is only "redundant" if one can live with the printing of the superfluous output of "Looking at blockwise erase function 0... not defined." etc.
if i add an eraser with a block size but no erase function i get: Erasing and writing flash chip... Looking at blockwise erase function 0... eraseblock layout is known, but matching block erase function is not implemented. trying... 0x000000-0x007fff:ESegmentation fault
so we may want to add a erasefn != NULL check to erase_and_write_block_helper? or even in the startup self checker (to look for erasers with blocks spreading > 0 bytes, but without a erase function)?
anyway this does not seem to be in the scope of this patch at all? i was under the assumption all the time that you thought it was.