To fix the below build warning: Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110:130: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] The second argument for NLMSG_OK need to be unsigned because it is compared with 'unsigned int'. http://lxr.free-electrons.com/source/include/linux/netlink.h#L86 41 struct nlmsghdr { 42 __u32 nlmsg_len; /* Length of message including header */ 43 __u16 nlmsg_type; /* Message content */ 44 __u16 nlmsg_flags; /* Additional flags */ 45 __u32 nlmsg_seq; /* Sequence number */ 46 __u32 nlmsg_pid; /* Sending process port ID */ 47 }; 86 #define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \ 87 (nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \ 88 (nlh)->nlmsg_len <= (len))
Created attachment 173850 [details] Patch
Comment on attachment 173850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173850&action=review > Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 > + while ((NLMSG_OK(nlh, (unsigned) len)) && (nlh->nlmsg_type != NLMSG_DONE)) { len is ssize_t type and it includes -1 in its value range. Why don't you insert some ASSERT stuff before here to make sure '-1' case is managed.
Comment on attachment 173850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173850&action=review >> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 >> + while ((NLMSG_OK(nlh, (unsigned) len)) && (nlh->nlmsg_type != NLMSG_DONE)) { > > len is ssize_t type and it includes -1 in its value range. > Why don't you insert some ASSERT stuff before here to make sure '-1' case is managed. In here, len always >0 because the outer while check for that. So, I didn't add additional code to check that for not making redundant code.
Comment on attachment 173850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173850&action=review >>> Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 >>> + while ((NLMSG_OK(nlh, (unsigned) len)) && (nlh->nlmsg_type != NLMSG_DONE)) { >> >> len is ssize_t type and it includes -1 in its value range. >> Why don't you insert some ASSERT stuff before here to make sure '-1' case is managed. > > In here, len always >0 because the outer while check for that. > So, I didn't add additional code to check that for not making redundant code. Ah, right! LGTM, thanks!
Comment on attachment 173850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=173850&action=review > Source/WebCore/ChangeLog:8 > + The second argument for NLMSG_OK need to be unsigned for fixing the -Wsign-compare warning. need -> needs for fixing --> to avoid > Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 > + while ((NLMSG_OK(nlh, (unsigned) len)) && (nlh->nlmsg_type != NLMSG_DONE)) { static_cast<unsigned>(len)
Created attachment 174048 [details] Patch
Comment on attachment 174048 [details] Patch Clearing flags on attachment: 174048 Committed r134537: <http://trac.webkit.org/changeset/134537>
All reviewed patches have been landed. Closing bug.
Comment on attachment 174048 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174048&action=review > Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 > + while ((NLMSG_OK(nlh, static_cast<unsigned>(len))) && (nlh->nlmsg_type != NLMSG_DONE)) { If I check "man netlink" on my machine, I get: int NLMSG_OK(struct nlmsghdr *nlh, int len); It says that the second argument is signed.
(In reply to comment #9) > (From update of attachment 174048 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174048&action=review > > > Source/WebCore/platform/network/efl/NetworkStateNotifierEfl.cpp:110 > > + while ((NLMSG_OK(nlh, static_cast<unsigned>(len))) && (nlh->nlmsg_type != NLMSG_DONE)) { > > If I check "man netlink" on my machine, I get: > int NLMSG_OK(struct nlmsghdr *nlh, int len); > > It says that the second argument is signed. Oh, I have just seen your first comment. the "man" page is misleading then. Clearly an issue with that macro but anyways.