Re: [PATCH 04/14] libfdt: fdt_add_string_(): Fix comparison warning

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

On Mon, Sep 21, 2020 at 05:52:53PM +0100, Andre Przywara wrote:
> With -Wsign-compare, compilers warn about a mismatching signedness
> in a comparison in fdt_add_string_().
> 
> As struct_top can only be positive, just use an unsigned type for it,
> and avoid the signedness difference.
> 
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>

I'm not sure this is right.  Well.. I'm also not sure it was right
before.  Adding some more context to explain why..

> ---
>  libfdt/fdt_sw.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
> index d10a720..d65e9c8 100644
> --- a/libfdt/fdt_sw.c
> +++ b/libfdt/fdt_sw.c
> @@ -249,7 +249,8 @@ static int fdt_add_string_(void *fdt, const char *s)
>  	char *strtab = (char *)fdt + fdt_totalsize(fdt);
>  	int strtabsize = fdt_size_dt_strings(fdt);
>  	int len = strlen(s) + 1;
> -	int struct_top, offset;
> +	unsigned int struct_top;
> +	int offset;
>  
>  	offset = -strtabsize - len;
>  	struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
>	if (fdt_totalsize(fdt) + offset < struct_top)
>		return 0; /* no more room :( */

So strtabsize and len will always be positive (or, if they're not,
that's another problem), so offset is always negative.  Which means we
need the signed addition between totalsize and offset for this to be
correct.

So I suspect we want to make 'len' and 'offset' unsigned as well,
reverse the sign on offset and make it a subtraction in the if instead
of an addition-of-negative.

We might then need to explicitly check for offset < totalsize as well,
to cover the overflow case.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]