Re: [Minios-devel] [UNIKRAFT/LWIP PATCH] Fix compilation warning about comparison between signed and unsigned integer

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 11/10/2019 09:55, Jia He wrote:
> Without this patch, compiler(gcc version 7.4.0) reports:
> proto.c: In function ‘getprotoent’:
> proto.c:85:10: warning: comparison between signed and unsigned integer
> expressions [-Wsign-compare]
>    if (idx >= sizeof(protos))
>            ^~
>
> Signed-off-by: Jia He <justin.he@xxxxxxx>
> ---
>   proto.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/proto.c b/proto.c
> index 517f258..7d32bc6 100644
> --- a/proto.c
> +++ b/proto.c
> @@ -82,7 +82,7 @@ struct protoent *getprotoent(void)
>         static struct protoent p;
>         static const char *aliases;
>   
> -	if (idx >= sizeof(protos))
> +       if ((unsigned int)idx >= sizeof(protos))
I am always cautious when I see an explicit cast.

If 'idx' is signed then it may mean it can be negative. Both the implicit and 
explicit cast version will pass the check for negative value. But, IMHO, this is 
a bad side-effect.
Looking at the code, I can't see a good reason for 'idx' to be signed. So why 
not switch to size_t?
I suggested size_t and not unsigned int because you increment it by strlen(..) 
which return a size_t.
>                 return NULL;
>         p.p_proto = protos[idx];
>         p.p_name = (char *)&protos[idx+1];
Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel