[Swan-dev] some warnings suppressed by pragma's (fwd)

Paul Wouters paul at nohats.ca
Tue Jun 9 00:28:08 EEST 2015

Forwarded to swan-dev

---------- Forwarded message ----------
Date: Mon, 8 Jun 2015 17:16:13
From: D. Hugh Redelmeier <hugh at mimosa.com>
To: Paul Wouters <paul at nohats.ca>
Subject: Re: some warnings suppressed by pragma's

Why isn't this on the dev list?

|  From: Paul Wouters <paul at nohats.ca>

|  Attached a smal patch that Len is proposing to fix building on his
|  debian server.
|  We've never done this pragma's push/pop, so I'm not sure if we
|  should do these. Looks like probably wrong band-aids.

Sadly, I think that these first two are probably the right band-aids.
The problem turns out to be way upstream.  I'd try to narrow their
application to JUST the NLMSG_OK call, at the cost of some ugliness.

I'm going to work at sending a patch to Linus.  Or maybe Lennart
should, since he worked it through.

 	 diff -urN libreswan-3.14rc1/programs/addconn/addconn.c
 	 --- libreswan-3.14rc1/programs/addconn/addconn.c	2015-06-03
 	 14:33:52.000000000 -0400
 	 +++ libreswan-3.14~rc1/programs/addconn/addconn.c	2015-06-08
 	 11:49:58.838269454 -0400
 	 @@ -157,8 +157,11 @@

 			 /* Verify it's valid */
 			 nlhdr = (struct nlmsghdr *) buf;
 	 +#pragma GCC diagnostic push
 	 +#pragma GCC diagnostic ignored "-Wsign-compare"
 			 if (NLMSG_OK(nlhdr, readlen) == 0 ||
 	 			nlhdr->nlmsg_type == NLMSG_ERROR)
 	 +#pragma GCC diagnostic pop

NETLINK(3) documents the second arg of NLMSG_OK, called len, to be of
type (signed) int.  This isn't very sensible, but it is the contract.

Looking at NLMSG_OK and the rest of /usr/include/linux/netlink.h, I
think it is that the header is broken. BTW, broken in Fedora 21 too.
And in Linus' kernel.

Our call:
 	NLMSG_OK(nlhdr, readlen) == 0
should be:
 	!NLMSG_OK(nlhdr, readlen)
because NLMSG_OK is a test that returns a boolean result.

NLMSG_OK includes a test
 	(len) >= (int)sizeof(struct nlmsghdr)
It suggests that len should be (signed)int.

In any case, in our call, len is readlen which is of type ssize_t, a
signed version of size_t.  So it should not be the problem for this
part of the test.

NLMSG_OK also contains the conjunct:
 	(nlh)->nlmsg_len <= (len)
The left size is __u32 and the right side is ssize_t.  So len cannot
represent all possible values of type __u32.  A good reason for a

So we cannot make the macro call avoid this diagnostic.

So: debian's NLMSG_OK macro has a bug.  It is obvious what the correct
fix to the header is:
 	(nlh)->nlmsg_len <= (len)
should be:
 	 (int)(nlh)->nlmsg_len <= (len)

 	 @@ -352,7 +355,10 @@
 		 /* Parse reply */
 		 struct nlmsghdr *nlmsg = (struct nlmsghdr *)msgbuf;

 	 +#pragma GCC diagnostic push
 	 +#pragma GCC diagnostic ignored "-Wsign-compare"
 	 	for (; NLMSG_OK(nlmsg, len); nlmsg = NLMSG_NEXT(nlmsg, len)) {
 	 +#pragma GCC diagnostic pop

Same problem.

 			 struct rtmsg *rtmsg;
 			 struct rtattr *rtattr;
 			 int rtlen;
 	 diff -urN libreswan-3.14rc1/programs/pluto/ikev1_spdb_struct.c
 	 --- libreswan-3.14rc1/programs/pluto/ikev1_spdb_struct.c
 	 2015-06-03 14:33:52.000000000 -0400
 	 +++ libreswan-3.14~rc1/programs/pluto/ikev1_spdb_struct.c
 	 2015-06-08 11:29:46.509673654 -0400
 	 @@ -1623,7 +1623,7 @@
 			 pb_stream attr_pbs;
 			 enum_names *vdesc;
 			 u_int16_t ty;
 	 -		u_int32_t val;                          /* room for
 	 larger value */
 	 +		int32_t val;                          /* room for
 	 larger value */
 			 bool ipcomp_inappropriate = (proto == PROTO_IPCOMP);
 			 /* will get reset if OK */

I don't know what this comes from, but it looks like the wrong change.
Values of val are definitely unsigned.

Perhaps it comes from one of our uses of val.  Which one(s)?

More information about the Swan-dev mailing list