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

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

[Phil Sutter]

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

[iproute PATCH v2 1/7] devlink: No need for this self-assignment

[Phil Sutter]

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

Re: [iproute PATCH v2 1/7] devlink: No need for this self-assignment

[Jiri Pirko]

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 <jiri@mellanox.com>

Re: [iproute PATCH v2 1/7] devlink: No need for this self-assignment

[Phil Sutter]

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:
> >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

Re: [iproute PATCH v2 1/7] devlink: No need for this self-assignment

[Jiri Pirko]

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:
>> 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.
>
>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

[iproute PATCH v2 2/7] ipntable: No need to check and assign to parms_rta

[Phil Sutter]

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

[iproute PATCH v2 3/7] iproute: Fix for missing 'Oifs:' display

[Phil Sutter]

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

[iproute PATCH v2 4/7] lib/rt_names: Drop dead code in rtnl_rttable_n2a()

[Phil Sutter]

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

[iproute PATCH v2 5/7] ss: Skip useless check in parse_hostcond()

[Phil Sutter]

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

[iproute PATCH v2 6/7] ss: Drop useless assignment

[Phil Sutter]

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

[iproute PATCH v2 7/7] tc/m_gact: Drop dead code

[Phil Sutter]

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

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

[Stephen Hemminger]

On Thu, 17 Aug 2017 19:09:24 +0200
Phil Sutter <phil@nwl.cc> 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.