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

["Justin He (Arm Technology China)" <Justin.He@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: Friday, October 11, 2019 5:28 PM
> To: Justin He (Arm Technology China) <Justin.He@xxxxxxx>; minios-
> devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer <simon.kuenzer@xxxxxxxxx>;
> Costin Lupu <costin.lupu@xxxxxxxxx>
> Cc: Sharan.Santhanam@xxxxxxxxx; Felipe Huici <felipe.huici@xxxxxxxxx>;
> Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>;
> Santiago.Pagani@xxxxxxxxx; nd <nd@xxxxxxx>
> Subject: Re: [UNIKRAFT/LWIP PATCH] Fix compilation warning about
> comparison between signed and unsigned integer
> 
> 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.

Okay, it is reasonable.
Thanks


--
Cheers,
Justin (Jia He)


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