[flashrom] [PATCH] Abort on unused programmer parameters

[Carl-Daniel Hailfinger]

Am 12.08.2013 00:53 schrieb Stefan Tauner:
> On Sat, 10 Aug 2013 17:43:15 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>> An unused programmer parameter is a sign that the user wanted to either
>> do something not supported by the programmer or misspelled a parameter
>> which may be essential for the given programmer. Aborting is the only
>> safe choice.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>
>> 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,14 @@
>>  	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. */
>> +		msg_perr("Unhandled programmer parameters: %s\n", programmer_param);
>> +		msg_perr("Aborting.\n");
> Considering pro and contra arguments for (not) combining these two
> lines, I am fine with it.</blah :)>
>
>> +		/* Do not overwrite any error code from programmer init. */
>> +		if (!ret)
>> +			ret = ERROR_FATAL;
>>  	}
>>  	return ret;
>>  }
>>
> IMHO the if is unnecessary, not beneficial and should be gone.
> In any case it is 
> Acked-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> 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 <c-d.hailfinger.devel.2006 at gmx.net>

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) {
+			/* 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;
 }

-- 
http://www.hailfinger.org/