Am Montag, den 28.06.2010, 15:35 +0200 schrieb Carl-Daniel Hailfinger:
+void nonfatal_help_message(void) +{
- msg_gerr("Writing to the flash chip apparently didn't do anything.\n"
"Trying to write to the flash chip..."
Hm. An earlier message says "writing", not "trying to write". With your change, we'd see messages like this:
Writing flash chip... Trying to write to the flash chip apparently didn't do anything.
That looks odd. I agree that my proposed message wasn't really good style, but the replacement isn't perfect either. Maybe a native speaker can comment.
Hmm. While trying to defend my position, I think your message isn't as bad as is initially thought. I think it would be confusing to talk about having "written" something and in the same sentence tell that this did not do anything. Actually, nothing was written to the memory, even if you sent write cycles to the chipset intending they end up in the chip and get forwarded to the array. The average user the "soft" error message is intended for doesn't know about the write cycle stuff, but will be happy to know that nothing was written.
"-------------------------------------------------------------""------------------\n""You may now reboot or simply leave the machine running.\n");"As nothing changed, powering off or rebooting the machine is not dangerous".
Should this message be printed in addition to or as replacement for "You may now reboot..."?
Replacement. I dislike the explicit license to "leave the machine running". Of *course* I can leave the machine running, as long until it crashes, no matter whether the flash chip is OK. What you want to say, if I understand it correctly, is that rebooting is safe, but it is not needed to reboot now. I think my suggestion makes it more clear.
How should we tell the user that rebooting is unsafe if a previous write failed? Example scenario: [...]
Excellent point, but it still applies to both the original and my reworded message. Something like "If you had no flashrom failures before, powering off or rebooting the machine is not dangerous, as the contents of your flash chip kept unchange in this run of flashrom".
msg_cerr("FAILED!\n");
msg_cerr("Uh oh. Write failed. Checking if anything ""changed.\n");/* FIXME: Should we really reuse buf here? */if (!flash->read(flash, buf, 0, size)) {if (!memcmp(oldflashcontents, buf, size)) {msg_cinfo("Good. It seems nothing was ""changed.\n");nonfatal_help_message();programmer_shutdown();return 1;}}Wait a moment... How can the chip be unmodified after passing an erase check? Only if it was empty before - no need to panic in that case. I suggest to remove the "did anything change" check from this piece of code.
What about a partially failed write? If the chip was erased before, the erase check will pass.
Right.
If write fails completely, the chip is still erased.
And it would only trigger the "soft" error message at this point with your patch, without the patch, it will trigger the "hard" error message.
If write fails partially, you have a partially written chip.
In that case, both the old and the new version show the "hard" error message.
Total write failure is not a problem with already erased chips, but partial write failure is indeed a problem.
I am not arguing against verifying the write after sucessfull erase (think of old dying chips!), I just claim that the extra verify to print the "soft" error message is unneeded in this case and we should keep the old behaviour of always printing the hard error message on write failure. Think of reading SPI and the time consumed by that.
Did I understand your thoughts correctly or did you mean something else?
I didn't really understand what was your point was, but maybe you didn't get my intention.
Regards, Michael Karcher