[flashrom] [PATCH] Fix compilation with only one enabled programmer.

[Carl-Daniel Hailfinger]

On 21.11.2015 00:43, Stefan Tauner wrote:
> Compilers are free to use an unsigned data type for enumerations without
> negative elements. Our enumeration of all programmers is one such
> instance. Also, compilers will warn about comparisons of unsigned enum
> expressions < 0.
>
> If we enable only a single programmer then PROGRAMMER_INVALID will be 1.
> We do some run-time checks of loop counters of an unsigned enumaration
> type against PROGRAMMER_INVALID in the form of
>   if (p < PROGRAMMER_INVALID - 1)
> which get optimized to
>   if (p < 0)
> This makes the check always false and the compiler warnings compulsory.

Which gcc version is this?


> This patch adds #ifdefs guarding these checks if there is only one programmer
> enabled.
>
> ---
> One can enable a single programmer by running
> grep CONFIG_ Makefile |fgrep '?= yes'|grep -vi serprog |sed "s/ .*/=no/"|xargs make  CONFIG_SERPROG=yes
>
> Signed-off-by: Stefan Tauner <stefan.tauner at alumni.tuwien.ac.at>
> ---
>  flashrom.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/flashrom.c b/flashrom.c
> index c9c7e31..abb917b 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -1633,10 +1633,13 @@ void list_programmers(const char *delim)
>  	enum programmer p;
>  	for (p = 0; p < PROGRAMMER_INVALID; p++) {
>  		msg_ginfo("%s", programmer_table[p].name);
> +
> +#if PROGRAMMER_INVALID > 1
>  		if (p < PROGRAMMER_INVALID - 1)

Does the warning also disappear if you write
if ((PROGRAMMER_INVALID > 1) && (p < PROGRAMMER_INVALID - 1) )
instead? AFAICS that should placate gcc (since this is a constant false
expression for PROGRAMMER_INVALID==1) even with warnings switched on.


>  			msg_ginfo("%s", delim);
> +#endif
>  	}
> -	msg_ginfo("\n");	
> +	msg_ginfo("\n");
>  }
>  
>  void list_programmers_linebreak(int startcol, int cols, int paren)
> @@ -1668,6 +1671,7 @@ void list_programmers_linebreak(int startcol, int cols, int paren)
>  		}
>  		msg_ginfo("%s", pname);
>  		remaining -= pnamelen;
> +#if PROGRAMMER_INVALID > 1
>  		if (p < PROGRAMMER_INVALID - 1) {
>  			msg_ginfo(",");
>  			remaining--;
> @@ -1675,6 +1679,9 @@ void list_programmers_linebreak(int startcol, int cols, int paren)
>  			if (paren)
>  				msg_ginfo(")");
>  		}
> +#else
> +		msg_ginfo(")");

Please drop the complete #else block. It adds a ")" where none should
be. It doesn't just look strange in the code, I also verified the output.


> +#endif
>  	}
>  }
>  

Regards,
Carl-Daniel