On Tue, 13 Aug 2013 01:25:17 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 12.08.2013 00:53 schrieb Stefan Tauner: > > > IMHO the if is unnecessary, not beneficial and should be gone. > > In any case it is > > Acked-by: Stefan Tauner <[email protected]> > > and I want that in 0.9.7. > > Thanks! > > After an extensive review on IRC, this will hopefully be a proper > implementation of the agreed upon programmer behaviour. > > Signed-off-by: Carl-Daniel Hailfinger <[email protected]> > > Index: flashrom-abort_unused_programmer_param/flashrom.c > =================================================================== > --- flashrom-abort_unused_programmer_param/flashrom.c (Revision 1706) > +++ flashrom-abort_unused_programmer_param/flashrom.c (Arbeitskopie) > @@ -389,13 +389,23 @@ > programmer_may_write = 1; > > programmer_param = param; > - msg_pdbg("Initializing %s programmer\n", > - programmer_table[programmer].name); > + msg_pdbg("Initializing %s programmer\n", > programmer_table[programmer].name); > ret = programmer_table[programmer].init(); > if (programmer_param && strlen(programmer_param)) { > - msg_perr("Unhandled programmer parameters: %s\n", > - programmer_param); > - /* Do not error out here, the init itself was successful. */ > + if (ret != 0) { I would have swapped the if/else branches, i.e. if (ret == 0) /* hard error */ else /* possibly due to another failure */, but that's just my weird taste probably :) > + /* It is quite possible that any unhandled programmer > parameter would have been valid, > + * but an error in actual programmer init happened > before the parameter was evaluated. > + */ > + msg_pwarn("Unhandled programmer parameters (possibly > due to another failure): %s\n", > + programmer_param); > + } else { > + /* Actual programmer init was successful, but the user > specified an invalid or unusable > + * (for the current programmer configuration) parameter. > + */ > + msg_perr("Unhandled programmer parameters: %s\n", > programmer_param); > + msg_perr("Aborting.\n"); > + ret = ERROR_FATAL; > + } > } > return ret; I can not imagine how this could possibly break anything in vanilla flashrom. I have also tested it with the dummy programmer: $ ./flashrom -p dummy:emulate=M25P10.RES,fail flashrom v0.9.6.1-r1707 on Linux 3.8.0-6-generic (x86_64) flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OK. Unhandled programmer parameters: fail Aborting. Error: Programmer initialization failed. $ ./flashrom -p dummy:emulate=initfail,image=persistent.img flashrom v0.9.6.1-r1707 on Linux 3.8.0-6-generic (x86_64) flashrom is free software, get the source code at http://www.flashrom.org Calibrating delay loop... OK. Invalid chip specified for emulation: initfail Unhandled programmer parameters (possibly due to another failure): image=persistent.img Error: Programmer initialization failed. Hence... Acked-by: Stefan Tauner <[email protected]> -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
