[iproute PATCH v2 0/7] Covscan: Dead code elimination

This series collects patches from v1 which deal with dead code, either
by removing it or changing context so it is accessed again if that makes
sense.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (7):
  devlink: No need for this self-assignment
  ipntable: No need to check and assign to parms_rta
  iproute: Fix for missing 'Oifs:' display
  lib/rt_names: Drop dead code in rtnl_rttable_n2a()
  ss: Skip useless check in parse_hostcond()
  ss: Drop useless assignment
  tc/m_gact: Drop dead code

 devlink/devlink.c |  2 +-
 ip/ipntable.c     |  2 --
 ip/iproute.c      |  8 +++++---
 lib/rt_names.c    |  4 ----
 misc/ss.c         |  3 +--
 tc/m_gact.c       | 14 +++-----------
 6 files changed, 10 insertions(+), 23 deletions(-)

-- 
2.13.1

After '*b = *a', 'b->next' already has the same value as 'a->next'.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 2debccce5260b..b2a7f069e294c 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1440,7 +1440,6 @@ static int remember_he(struct aafilter *a, struct hostent *he)
 			if ((b = malloc(sizeof(*b))) == NULL)
 				return cnt;
 			*b = *a;
-			b->next = a->next;
 			a->next = b;
 		}
 		memcpy(b->addr.data, *ptr, len);

-- 
2.13.1

Covscan complained about dead code but after reading it, I assume the
author's intention was to prefix the interface list with 'Oifs: '.
Initializing first to 1 and setting it to 0 after above prefix was
printed should fix it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/iproute.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index cb695ad4141a7..89caac124f489 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c

@@ -624,7 +624,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	}
 	if (tb[RTA_MULTIPATH]) {
 		struct rtnexthop *nh = RTA_DATA(tb[RTA_MULTIPATH]);
-		int first = 0;
+		int first = 1;
 
 		len = RTA_PAYLOAD(tb[RTA_MULTIPATH]);
 

@@ -634,10 +634,12 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			if (nh->rtnh_len > len)
 				break;
 			if (r->rtm_flags&RTM_F_CLONED && r->rtm_type == RTN_MULTICAST) {
-				if (first)
+				if (first) {
 					fprintf(fp, "Oifs: ");
-				else
+					first = 0;
+				} else {
 					fprintf(fp, " ");
+				}
 			} else
 				fprintf(fp, "%s\tnexthop ", _SL_);
 			if (nh->rtnh_len > sizeof(*nh)) {

-- 
2.13.1

dl_argv_handle_both() will either assign to handle_bit or error out in
which case the variable is not used by the caller.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 devlink/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index f9bc16c350c40..bf43e2cd5e709 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c

@@ -779,7 +779,7 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 	int err;
 
 	if (o_required & DL_OPT_HANDLE && o_required & DL_OPT_HANDLEP) {
-		uint32_t handle_bit = handle_bit;
+		uint32_t handle_bit;
 
 		err = dl_argv_handle_both(dl, &opts->bus_name, &opts->dev_name,
 					  &opts->port_index, &handle_bit);

-- 
2.13.1

The passed 'addr' parameter is dereferenced by caller before and in
parse_hostcond() multiple times before this check, so assume it is
always true.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index f0d1c22f75cff..2debccce5260b 100644
--- a/misc/ss.c
+++ b/misc/ss.c

@@ -1671,7 +1671,7 @@ void *parse_hostcond(char *addr, bool is_port)
 			}
 		}
 	}
-	if (!is_port && addr && *addr && *addr != '*') {
+	if (!is_port && *addr && *addr != '*') {
 		if (get_prefix_1(&a.addr, addr, fam)) {
 			if (get_dns_host(&a, addr, fam)) {
 				fprintf(stderr, "Error: an inet prefix is expected rather than \"%s\".\n", addr);

-- 
2.13.1

The use of 'ok' variable in parse_gact() is ineffective: The second
conditional increments it either if *argv is 'gact' or if
parse_action_control() doesn't fail (in which case exit() is called).
So this is effectively an unconditional increment and since no decrement
happens anywhere, all remaining checks for 'ok != 0' can be dropped.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_gact.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/tc/m_gact.c b/tc/m_gact.c
index 1a2583372c34e..df143c9e0953e 100644
--- a/tc/m_gact.c
+++ b/tc/m_gact.c

@@ -76,7 +76,6 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
 {
 	int argc = *argc_p;
 	char **argv = *argv_p;
-	int ok = 0;
 	struct tc_gact p = { 0 };
 #ifdef CONFIG_GACT_PROB
 	int rd = 0;

@@ -89,17 +88,14 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
 
 
 	if (matches(*argv, "gact") == 0) {
-		ok++;
 		argc--;
 		argv++;
-	} else {
-		if (parse_action_control(&argc, &argv, &p.action, false) == -1)
-			usage();
-		ok++;
+	} else if (parse_action_control(&argc, &argv, &p.action, false) == -1) {
+		usage();	/* does not return */
 	}
 
 #ifdef CONFIG_GACT_PROB
-	if (ok && argc > 0) {
+	if (argc > 0) {
 		if (matches(*argv, "random") == 0) {
 			rd = 1;
 			NEXT_ARG();

@@ -142,15 +138,11 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p,
 			}
 			argc--;
 			argv++;
-			ok++;
 		} else if (matches(*argv, "help") == 0) {
 				usage();
 		}
 	}
 
-	if (!ok)
-		return -1;
-
 	tail = NLMSG_TAIL(n);
 	addattr_l(n, MAX_MSG, tca_id, NULL, 0);
 	addattr_l(n, MAX_MSG, TCA_GACT_PARMS, &p, sizeof(p));

-- 
2.13.1

This variable is initialized at declaration and nowhere else does any
assignment to it happen, so just drop the check.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipntable.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/ip/ipntable.c b/ip/ipntable.c
index 7be1f04d33d90..30907146e85a3 100644
--- a/ip/ipntable.c
+++ b/ip/ipntable.c

@@ -202,8 +202,6 @@ static int ipntable_modify(int cmd, int flags, int argc, char **argv)
 			if (get_u32(&queue, *argv, 0))
 				invarg("\"queue\" value is invalid", *argv);
 
-			if (!parms_rta)
-				parms_rta = (struct rtattr *)&parms_buf;
 			rta_addattr32(parms_rta, sizeof(parms_buf),
 				      NDTPA_QUEUE_LEN, queue);
 			parms_change = 1;

-- 
2.13.1

Since 'id' is 32bit unsigned, it can never exceed RT_TABLE_MAX (which is
defined to 0xFFFFFFFF). Therefore drop that never matching conditional.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 lib/rt_names.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lib/rt_names.c b/lib/rt_names.c
index 04c15ff5b15f8..e5efd78e6f810 100644
--- a/lib/rt_names.c
+++ b/lib/rt_names.c

@@ -410,10 +410,6 @@ const char *rtnl_rttable_n2a(__u32 id, char *buf, int len)
 {
 	struct rtnl_hash_entry *entry;
 
-	if (id > RT_TABLE_MAX) {
-		snprintf(buf, len, "%u", id);
-		return buf;
-	}
 	if (!rtnl_rttable_init)
 		rtnl_rttable_initialize();
 	entry = rtnl_rttable_hash[id & 255];

-- 
2.13.1

Thu, Aug 17, 2017 at 07:09:25PM CEST, phil@nwl.cc wrote:

dl_argv_handle_both() will either assign to handle_bit or error out in
which case the variable is not used by the caller.

I'm pretty sure that I did this to silence the compiler. If the compiler
bug is fixed now, good.

Acked-by: Jiri Pirko <redacted>

On Thu, Aug 17, 2017 at 09:48:50PM +0200, Jiri Pirko wrote:

Thu, Aug 17, 2017 at 07:09:25PM CEST, phil@nwl.cc wrote:

quoted

dl_argv_handle_both() will either assign to handle_bit or error out in
which case the variable is not used by the caller.

I'm pretty sure that I did this to silence the compiler. If the compiler
bug is fixed now, good.

That might depend on the compiler you used, so maybe you just want to
give it a try in your environment? If it still happens, we can keep this
self-assignment of course since it shouldn't harm.

Thanks, Phil

Fri, Aug 18, 2017 at 12:20:24PM CEST, phil@nwl.cc wrote:

On Thu, Aug 17, 2017 at 09:48:50PM +0200, Jiri Pirko wrote:

quoted

Thu, Aug 17, 2017 at 07:09:25PM CEST, phil@nwl.cc wrote:

quoted

dl_argv_handle_both() will either assign to handle_bit or error out in
which case the variable is not used by the caller.

I'm pretty sure that I did this to silence the compiler. If the compiler
bug is fixed now, good.

That might depend on the compiler you used, so maybe you just want to
give it a try in your environment? If it still happens, we can keep this
self-assignment of course since it shouldn't harm.

No warning with gcc 6.3.1

On Thu, 17 Aug 2017 19:09:24 +0200
Phil Sutter [off-list ref] wrote:

This series collects patches from v1 which deal with dead code, either
by removing it or changing context so it is accessed again if that makes
sense.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (7):
  devlink: No need for this self-assignment
  ipntable: No need to check and assign to parms_rta
  iproute: Fix for missing 'Oifs:' display
  lib/rt_names: Drop dead code in rtnl_rttable_n2a()
  ss: Skip useless check in parse_hostcond()
  ss: Drop useless assignment
  tc/m_gact: Drop dead code

 devlink/devlink.c |  2 +-
 ip/ipntable.c     |  2 --
 ip/iproute.c      |  8 +++++---
 lib/rt_names.c    |  4 ----
 misc/ss.c         |  3 +--
 tc/m_gact.c       | 14 +++-----------
 6 files changed, 10 insertions(+), 23 deletions(-)

Sure these look fine. Applied.