[Swan-dev] netlink.h:NLMSG_OK change to avoid GCC warnings

[D. Hugh Redelmeier]

[I want to send this to the kernel API developers.  Does it look 
reasonable?]

[Can someone test on a 32-bit system where libreswan addcon.c got build 
warnings?  That would involve testing with and without the hacky casts to 
unsigned short I added to the second argument of the two calls to NLMSG_OK 
in addcon.c.  All appears well in my 64-bit system.]

Summary: please add a cast to avoid a GCC warning on 32-bit userland.

GCC seems to have added a new warning that netlink.h provokes on i386.
netlink.h should be changed to avoid this, even though the netlink.h
code is correct.

GCC's warning is to flag comparisons where the implicit "Usual
Arithmetic Conversions" could lead to a surprising result.

Consider this context:
	int i;
	unsigned u;
The C standard says that i<u is evaluated as (unsigned)i < u.
This can easily be a surprise because a negative value of i will be 
silently treated as a very large positive value.

In include/uapi/linux/netlink.h:
#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
			   (nlh)->nlmsg_len <= (len))

This comparison looks suspicious to GCC:
	(nlh)->nlmsg_len <= (len)
nlmsg_len is of type __u32.  If int is 32 bits, this compare will be
done as
	(nlh)->nlmsg_len <= (unsigned)(len)
We know that this is actually safe because the first conjunct
determined that len isn't negative but the GCC apparently doesn't know.

A change that would calm GCC and also be correct would be to add
a cast to:
	(nlh)->nlmsg_len <= (unsigned)(len))
But I imagine that len might well actually have type ssize_t.  It is
often the result of a call to recvfrom(2), which is a ssize_t.  So I
think that this would be safer:
	(nlh)->nlmsg_len <= (size_t)(len))
I know of no system where size_t is narrower than unsigned.

This problem came up when building a userland component of libreswan in a 
32-bit environment with a recent GCC and was reported by Lennart Sorensen.

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index cf6a65c..0343f58 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -86,7 +86,7 @@ struct nlmsghdr {
 				  (struct nlmsghdr*)(((char*)(nlh)) + NLMSG_ALIGN((nlh)->nlmsg_len)))
 #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
 			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
-			   (nlh)->nlmsg_len <= (len))
+			   (nlh)->nlmsg_len <= (size_t)(len))
 #define NLMSG_PAYLOAD(nlh,len) ((nlh)->nlmsg_len - NLMSG_SPACE((len)))
 
 #define NLMSG_NOOP		0x1	/* Nothing.		*/