Re: [PATCH 03/11] fdtdump: Fix signedness comparisons warnings

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

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

On Mon, Oct 12, 2020 at 05:19:40PM +0100, Andre Przywara wrote:
65;6003;1c> With -Wsign-compare, compilers warn about a mismatching signedness in
> comparisons in fdtdump.c.
> 
> The "len" parameter to valid_header() refers to a memory size, not a
> file offset, so the (unsigned) size_t is better fit, and fixes the
> warning nicely.
> 
> In the main function we compare the difference between two pointers,
> which produces a signed ptrdiff_t type. However the while loop above the
> comparison makes sure that "p" always points before "endp" (by virtue of
> the limit in the memchr() call). This means "endp - p" is never
> negative, so we can safely cast this expression to an unsigned type.
> 
> This fixes "make fdtdump", when compiled with -Wsign-compare.
> 
> Signed-off-by: Andre Przywara <andre.przywara-5wv7dgnIgG8@public.gmane.org>
> ---
>  fdtdump.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fdtdump.c b/fdtdump.c
> index 9613bef..ec67f99 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -18,7 +18,7 @@
>  #include "util.h"
>  
>  #define FDT_MAGIC_SIZE	4
> -#define MAX_VERSION 17
> +#define MAX_VERSION 17U
>  
>  #define ALIGN(x, a)	(((x) + ((a) - 1)) & ~((a) - 1))
>  #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
> @@ -163,7 +163,7 @@ static const char * const usage_opts_help[] = {
>  	USAGE_COMMON_OPTS_HELP
>  };
>  
> -static bool valid_header(char *p, off_t len)
> +static bool valid_header(char *p, size_t len)
>  {
>  	if (len < sizeof(struct fdt_header) ||
>  	    fdt_magic(p) != FDT_MAGIC ||
> @@ -235,7 +235,7 @@ int main(int argc, char *argv[])
>  			}
>  			++p;
>  		}
> -		if (!p || endp - p < sizeof(struct fdt_header))
> +		if (!p || (unsigned)(endp - p) < sizeof(struct fdt_header))

I think (size_t) would be more appropriate here, since it will match
the other size of the comparison, though it's very unlikely to cause a
problem in practice.

>  			die("%s: could not locate fdt magic\n", file);
>  		printf("%s: found fdt at offset %#tx\n", file, p - buf);
>  		buf = p;

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