[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 2/2] lib/uknetdev: Fix compliation warning about signed/unsigned comparision
On 11/10/2019 16:13, Justin He (Arm Technology China) wrote: Hi Julien Hi, -----Original Message----- From: Julien Grall <julien.grall@xxxxxxx> Sent: Friday, October 11, 2019 5:00 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 PATCH 2/2] lib/uknetdev: Fix compliation warning about signed/unsigned comparision Hi, On 11/10/2019 09:52, Jia He wrote:This fixes the compliation warning when compiling libuknetdevTypo s/compliation/compilation//root/hj/UK/unikraft/lib/uknetdev/netbuf.c: In function 'uk_netbuf_alloc_buf': /root/hj/UK/unikraft/lib/uknetdev/netbuf.c:120:35: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (likely(UINT16_MAX - headroom > NETBUF_ADDR_ALIGNMENT)) { ^ /root/hj/UK/unikraft/include/uk/arch/lcpu.h:48:43: note: in definition of macro 'likely' #define likely(x) (__builtin_expect((!!(x)), 1)) ^ Signed-off-by: Jia He <justin.he@xxxxxxx> --- lib/uknetdev/netbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/uknetdev/netbuf.c b/lib/uknetdev/netbuf.c index 96a5f68..108bd36 100644 --- a/lib/uknetdev/netbuf.c +++ b/lib/uknetdev/netbuf.c @@ -36,7 +36,7 @@ #include <uk/print.h> /* Used to align netbuf's priv and data areas to `long long` data type */ -#define NETBUF_ADDR_ALIGNMENT (sizeof(long long)) +#define NETBUF_ADDR_ALIGNMENT (int)(sizeof(long long))Maybe I am missing something but: UINT16_MAX - headrom should be unsigned, right? IIRC, sizeof() is also returning an unsigned. So how this is solving the problem here?Because UINT16_MAX and headroom are both unsigned short, Looking at the definition, UINT16_MAX is an unsigned int. So I am a bit confused. Integer types smaller than int are promoted when an operation is performed on them. Thus, the left side of equation is "int", and the right side is size_t. It would be nice if you can provide more information in your commit message. This helps the reviewer to figure out the problem more easily. Although, I am still unsure to understand how this is related to the issue here. Wouldn't the problem be unsigned integer are promoted to long long? Regarding the patch itself, this is only deferring the problem until someone come up with a check that will use unsigned value. A better approach would be to fix it in the check directly. Maybe: (size_t)(UINT16_MAX - headroom)? Cheers, -- Julien Grall _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |