All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Ahern <dsahern@kernel.org>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, dan.carpenter@oracle.com,
	David Ahern <dsahern@gmail.com>
Subject: [PATCH net-next] selftests: Fix get_ifidx and callers in nettest.c
Date: Wed, 14 Aug 2019 10:11:51 -0700	[thread overview]
Message-ID: <20190814171151.27739-1-dsahern@kernel.org> (raw)

From: David Ahern <dsahern@gmail.com>

Dan reported:

    The patch acda655fefae: "selftests: Add nettest" from Aug 1, 2019,
    leads to the following static checker warning:

            ./tools/testing/selftests/net/nettest.c:1690 main()
            warn: unsigned 'tmp' is never less than zero.

    ./tools/testing/selftests/net/nettest.c
      1680                  case '1':
      1681                          args.has_expected_raddr = 1;
      1682                          if (convert_addr(&args, optarg,
      1683                                           ADDR_TYPE_EXPECTED_REMOTE))
      1684                                  return 1;
      1685
      1686                          break;
      1687                  case '2':
      1688                          if (str_to_uint(optarg, 0, 0x7ffffff, &tmp) != 0) {
      1689                                  tmp = get_ifidx(optarg);
      1690                                  if (tmp < 0) {

    "tmp" is unsigned so it can't be negative.  Also all the callers assume
    that get_ifidx() returns negatives on error but it looks like it really
    returns zero on error so it's a bit unclear to me.

Update get_ifidx to return -1 on errors and cleanup callers of it.

Fixes: acda655fefae ("selftests: Add nettest")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/testing/selftests/net/nettest.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
index 83515e5ea4dc..c08f4db8330d 100644
--- a/tools/testing/selftests/net/nettest.c
+++ b/tools/testing/selftests/net/nettest.c
@@ -266,7 +266,7 @@ static int get_ifidx(const char *ifname)
 	int sd, rc;
 
 	if (!ifname || *ifname == '\0')
-		return 0;
+		return -1;
 
 	memset(&ifdata, 0, sizeof(ifdata));
 
@@ -275,14 +275,14 @@ static int get_ifidx(const char *ifname)
 	sd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
 	if (sd < 0) {
 		log_err_errno("socket failed");
-		return 0;
+		return -1;
 	}
 
 	rc = ioctl(sd, SIOCGIFINDEX, (char *)&ifdata);
 	close(sd);
 	if (rc != 0) {
 		log_err_errno("ioctl(SIOCGIFINDEX) failed");
-		return 0;
+		return -1;
 	}
 
 	return ifdata.ifr_ifindex;
@@ -419,20 +419,20 @@ static int set_multicast_if(int sd, int ifindex)
 	return rc;
 }
 
-static int set_membership(int sd, uint32_t grp, uint32_t addr, const char *dev)
+static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
 {
 	uint32_t if_addr = addr;
 	struct ip_mreqn mreq;
 	int rc;
 
-	if (addr == htonl(INADDR_ANY) && !dev) {
+	if (addr == htonl(INADDR_ANY) && !ifindex) {
 		log_error("Either local address or device needs to be given for multicast membership\n");
 		return -1;
 	}
 
 	mreq.imr_multiaddr.s_addr = grp;
 	mreq.imr_address.s_addr = if_addr;
-	mreq.imr_ifindex = dev ? get_ifidx(dev) : 0;
+	mreq.imr_ifindex = ifindex;
 
 	rc = setsockopt(sd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &mreq, sizeof(mreq));
 	if (rc < 0) {
@@ -1048,7 +1048,7 @@ static int msock_init(struct sock_args *args, int server)
 
 	if (server &&
 	    set_membership(sd, args->grp.s_addr,
-			   args->local_addr.in.s_addr, args->dev))
+			   args->local_addr.in.s_addr, args->ifindex))
 		goto out_err;
 
 	return sd;
@@ -1685,15 +1685,16 @@ int main(int argc, char *argv[])
 
 			break;
 		case '2':
-			if (str_to_uint(optarg, 0, 0x7ffffff, &tmp) != 0) {
-				tmp = get_ifidx(optarg);
-				if (tmp < 0) {
+			if (str_to_uint(optarg, 0, INT_MAX, &tmp) == 0) {
+				args.expected_ifindex = (int)tmp;
+			} else {
+				args.expected_ifindex = get_ifidx(optarg);
+				if (args.expected_ifindex < 0) {
 					fprintf(stderr,
-						"Invalid device index\n");
+						"Invalid expected device\n");
 					return 1;
 				}
 			}
-			args.expected_ifindex = (int)tmp;
 			break;
 		case 'q':
 			quiet = 1;
-- 
2.11.0


             reply	other threads:[~2019-08-14 17:21 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 17:11 David Ahern [this message]
2019-08-16 22:25 ` [PATCH net-next] selftests: Fix get_ifidx and callers in nettest.c David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190814171151.27739-1-dsahern@kernel.org \
    --to=dsahern@kernel.org \
    --cc=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.