On Mon, Oct 12, 2020 at 05:19:41PM +0100, Andre Przywara wrote:
> With -Wsign-compare, compilers warn about a mismatching signedness in
> the different legs of the conditional operator, in fdtget.c.
> 
> In the questionable expression, we are constructing a 16-bit value out of
> two unsigned 8-bit values, however are relying on the compiler's
> automatic expansion of the uint8_t to a larger type, to survive the left
> shift. This larger type happens to be an "int", so this part of the
> expression becomes signed.
> 
> Fix this by explicitly blow up the uint8_t to a larger *unsigned* type,
> before doing the left shift. That keeps all parts of the conditional
> operator "unsigned", and prevents the original warning.
> 
> This fixes "make fdtget", when compiled with -Wsign-compare.
> 
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> ---
>  fdtget.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fdtget.c b/fdtget.c
> index 777582e..c5685d9 100644
> --- a/fdtget.c
> +++ b/fdtget.c
> @@ -62,8 +62,8 @@ static int show_cell_list(struct display_info *disp, const char *data, int len,
>  	for (i = 0; i < len; i += size, p += size) {
>  		if (i)
>  			printf(" ");
> -		value = size == 4 ? fdt32_ld((const fdt32_t *)p) :
> -			size == 2 ? (*p << 8) | p[1] : *p;
> +		value = (size == 4) ? fdt32_ld((const fdt32_t *)p) :
> +			((size == 2) ? ((unsigned)(*p) << 8) | p[1] : *p);

Oof, yeah, that's a complicated expression.  I wonder if we should add
an fdt16_ld() to simplify this.

>  		printf(fmt, value);
>  	}
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson