Patchwork Abort on unused programmer parameters

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2013-08-10 15:43:15
Message ID <52065F93.2030409@gmx.net>
Download mbox | patch
Permalink /patch/4018/
State Superseded
Headers show

Comments

Carl-Daniel Hailfinger - 2013-08-10 15:43:15
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@gmx.net>
Stefan Tauner - 2013-08-11 22:53:55
On Sat, 10 Aug 2013 17:43:15 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@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@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@student.tuwien.ac.at>
and I want that in 0.9.7.

Patch

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");
+		/* Do not overwrite any error code from programmer init. */
+		if (!ret)
+			ret = ERROR_FATAL;
 	}
 	return ret;
 }