Preparing for the next release: call for testing

[Dmitry V. Levin]

On Tue, Mar 03, 2015 at 01:23:53PM -0500, Mike Frysinger wrote:
> On 03 Mar 2015 21:01, Dmitry V. Levin wrote:
> > On Tue, Mar 03, 2015 at 12:47:19PM -0500, Mike Frysinger wrote:
> > > build warnings:
> > > there's still random -Wsign-compare warnings, but i guess we don't care
> > > about those
> > 
> > There are exactly 3 different -Wsign-compare warning messages:
> > socketutils.c:145: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > netlink_inet_diag.c:67: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > netlink_unix_diag.c:75: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> > 
> > The only way to fix them is to fix NLMSG_OK macro defined
> > in <linux/netlink.h>.
> 
> hmm, i'm not so sure.  the kernel headers declare:
> struct nlmsghdr {
> 	__u32		nlmsg_len;	/* Length of message including header */
> ...
> #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
> 			   (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
> 			   (nlh)->nlmsg_len <= (len))
> 
> and the strace code does:
> 		ssize_t ret;
> 		struct nlmsghdr *h;
> 		...
> 		     NLMSG_OK(h, ret);
> 
> if the kernel headers provided a function instead of a macro, it'd be:
> static bool NLMSG_OK(const struct nlmsghdr *nlh, __u32 len) {...}
> 
> which is to say, the API of this macro is that it takes a u32 len, but we're 
> passing it a ssize_t (as that is what recvmsg/etc... returns).  it is annoying 
> that there's a type mismatch here, but i think the problem is still on our side 
> to sort out:
> 	NLMSG_OK(h, (size_t)ret)

If we pass (size_t)ret to NLMSG_OK, it would result to
	((size_t)ret) >= (int)sizeof(struct nlmsghdr)
and this code generates the same -Wsign-compare warning.
I actually tried it some time ago.

> we've already verified earlier in the code that it's not negative, so the cast 
> by itself should be safe:
> 		ret = recvmsg(fd, &msg, 0);
> 		if (ret < 0) {
> 			if (errno == EINTR)
> 				continue;
> 			return false;
> 		}

Yes, in this case the warning is harmless.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20150303/7b83d39c/attachment.bin>