[PATCH iproute2-next 0/3] route code cleanups

[PATCH iproute2-next 0/3] route code cleanups

[Stephen Hemminger]

Make code in iproute conform more to current coding style
conventions. Also split flush out into separate function.

Stephen Hemminger (3):
  iproute: whitespace fixes
  iproute: don't do assignment in condition
  iproute: make flush a separate function

 ip/iproute.c | 165 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 91 insertions(+), 74 deletions(-)

-- 
2.15.1

[PATCH iproute2-next 1/3] iproute: whitespace fixes

[Stephen Hemminger]

Add whitespace around operators for consistency.
Use tabs for indentation.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iproute.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 91d2b1a61993..e9c4093fa3eb 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -162,7 +162,7 @@ static int filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 	if (r->rtm_family == AF_INET6 && table != RT_TABLE_MAIN)
 		ip6_multiple_tables = 1;
 
-	if (filter.cloned == !(r->rtm_flags&RTM_F_CLONED))
+	if (filter.cloned == !(r->rtm_flags & RTM_F_CLONED))
 		return 0;
 
 	if (r->rtm_family == AF_INET6 && !ip6_multiple_tables) {
@@ -566,7 +566,8 @@ static void print_rta_multipath(FILE *fp, const struct rtmsg *r,
 		if (nh->rtnh_len > len)
 			break;
 
-		if (r->rtm_flags&RTM_F_CLONED && r->rtm_type == RTN_MULTICAST) {
+		if ((r->rtm_flags & RTM_F_CLONED) &&
+		    r->rtm_type == RTN_MULTICAST) {
 			if (first) {
 				fprintf(fp, "Oifs: ");
 				first = 0;
@@ -594,7 +595,8 @@ static void print_rta_multipath(FILE *fp, const struct rtmsg *r,
 				print_rta_flow(fp, tb[RTA_FLOW]);
 		}
 
-		if (r->rtm_flags&RTM_F_CLONED && r->rtm_type == RTN_MULTICAST) {
+		if ((r->rtm_flags & RTM_F_CLONED) &&
+		    r->rtm_type == RTN_MULTICAST) {
 			fprintf(fp, "%s", ll_index_to_name(nh->rtnh_ifindex));
 			if (nh->rtnh_hops != 1)
 				fprintf(fp, "(ttl>%d)", nh->rtnh_hops);
@@ -676,7 +678,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_dst_len != host_len) {
 			fprintf(fp, "%s/%u ",
 				rt_addr_n2a_rta(family, tb[RTA_DST]),
-			        r->rtm_dst_len);
+				r->rtm_dst_len);
 		} else {
 			fprintf(fp, "%s ",
 				format_host_rta(family, tb[RTA_DST]));
@@ -691,7 +693,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		if (r->rtm_src_len != host_len) {
 			fprintf(fp, "from %s/%u ",
 				rt_addr_n2a_rta(family, tb[RTA_SRC]),
-			        r->rtm_src_len);
+				r->rtm_src_len);
 		} else {
 			fprintf(fp, "from %s ",
 				format_host_rta(family, tb[RTA_SRC]));
@@ -722,7 +724,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 
 	if (table && (table != RT_TABLE_MAIN || show_details > 0) && !filter.tb)
 		fprintf(fp, "table %s ", rtnl_rttable_n2a(table, b1, sizeof(b1)));
-	if (!(r->rtm_flags&RTM_F_CLONED)) {
+	if (!(r->rtm_flags & RTM_F_CLONED)) {
 		if ((r->rtm_protocol != RTPROT_BOOT || show_details > 0) && filter.protocolmask != -1)
 			fprintf(fp, "proto %s ", rtnl_rtprot_n2a(r->rtm_protocol, b1, sizeof(b1)));
 		if ((r->rtm_scope != RT_SCOPE_UNIVERSE || show_details > 0) && filter.scopemask != -1)
@@ -764,7 +766,6 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			print_rta_cacheinfo(fp, RTA_DATA(tb[RTA_CACHEINFO]));
 
 	} else if (r->rtm_family == AF_INET6) {
-
 		if (r->rtm_flags & RTM_F_CLONED)
 			fprintf(fp, "%s    cache ", _SL_);
 
@@ -1577,8 +1578,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 				invarg("invalid mark value", *argv);
 			filter.markmask = -1;
 		} else if (matches(*argv, "metric") == 0 ||
-		           matches(*argv, "priority") == 0 ||
-		           strcmp(*argv, "preference") == 0) {
+			   matches(*argv, "priority") == 0 ||
+			   strcmp(*argv, "preference") == 0) {
 			__u32 metric;
 
 			NEXT_ARG();
@@ -2117,6 +2118,8 @@ int do_iproute(int argc, char **argv)
 		return iproute_showdump();
 	if (matches(*argv, "help") == 0)
 		usage();
-	fprintf(stderr, "Command \"%s\" is unknown, try \"ip route help\".\n", *argv);
+
+	fprintf(stderr,
+		"Command \"%s\" is unknown, try \"ip route help\".\n", *argv);
 	exit(-1);
 }
-- 
2.15.1

[PATCH iproute2-next 2/3] iproute: don't do assignment in condition

[Stephen Hemminger]

Fix checkpatch complaints about assignment in conditions.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iproute.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index e9c4093fa3eb..22ed113e890a 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -653,7 +653,8 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 		struct nlmsghdr *fn;
 
 		if (NLMSG_ALIGN(filter.flushp) + n->nlmsg_len > filter.flushe) {
-			if ((ret = flush_update()) < 0)
+			ret = flush_update();
+			if (ret < 0)
 				return ret;
 		}
 		fn = (struct nlmsghdr *)(filter.flushb + NLMSG_ALIGN(filter.flushp));
@@ -827,7 +828,8 @@ static int parse_one_nh(struct nlmsghdr *n, struct rtmsg *r,
 			}
 		} else if (strcmp(*argv, "dev") == 0) {
 			NEXT_ARG();
-			if ((rtnh->rtnh_ifindex = ll_name_to_index(*argv)) == 0) {
+			rtnh->rtnh_ifindex = ll_name_to_index(*argv);
+			if (rtnh->rtnh_ifindex == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n", *argv);
 				return -1;
 			}
@@ -1326,9 +1328,9 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 		usage();
 
 	if (d) {
-		int idx;
+		int idx = ll_name_to_index(d);
 
-		if ((idx = ll_name_to_index(d)) == 0) {
+		if (idx == 0) {
 			fprintf(stderr, "Cannot find device \"%s\"\n", d);
 			return -1;
 		}
@@ -1658,7 +1660,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 		int idx;
 
 		if (id) {
-			if ((idx = ll_name_to_index(id)) == 0) {
+			idx = ll_name_to_index(id);
+			if (idx == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n", id);
 				return -1;
 			}
@@ -1666,7 +1669,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 			filter.iifmask = -1;
 		}
 		if (od) {
-			if ((idx = ll_name_to_index(od)) == 0) {
+			idx = ll_name_to_index(od);
+			if (idx == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n", od);
 				return -1;
 			}
@@ -1716,7 +1720,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 				return 0;
 			}
 			round++;
-			if ((ret = flush_update()) < 0)
+			ret = flush_update();
+			if (ret < 0)
 				return ret;
 
 			if (time(0) - start > 30) {
@@ -1867,14 +1872,16 @@ static int iproute_get(int argc, char **argv)
 		int idx;
 
 		if (idev) {
-			if ((idx = ll_name_to_index(idev)) == 0) {
+			idx = ll_name_to_index(idev);
+			if (idx == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n", idev);
 				return -1;
 			}
 			addattr32(&req.n, sizeof(req), RTA_IIF, idx);
 		}
 		if (odev) {
-			if ((idx = ll_name_to_index(odev)) == 0) {
+			idx = ll_name_to_index(odev);
+			if (idx == 0) {
 				fprintf(stderr, "Cannot find device \"%s\"\n", odev);
 				return -1;
 			}
-- 
2.15.1

[PATCH iproute2-next 3/3] iproute: make flush a separate function

[Stephen Hemminger]

Minor refactoring to move flush into separate function to improve
readability and reduce depth of nesting.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 ip/iproute.c | 121 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 64 insertions(+), 57 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index 22ed113e890a..3c56240f1291 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1473,6 +1473,68 @@ static int save_route_prep(void)
 	return 0;
 }
 
+static int iproute_flush(int do_ipv6, rtnl_filter_t filter_fn)
+{
+	time_t start = time(0);
+	char flushb[4096-512];
+	int round = 0;
+	int ret;
+
+	if (filter.cloned) {
+		if (do_ipv6 != AF_INET6) {
+			iproute_flush_cache();
+			if (show_stats)
+				printf("*** IPv4 routing cache is flushed.\n");
+		}
+		if (do_ipv6 == AF_INET)
+			return 0;
+	}
+
+	filter.flushb = flushb;
+	filter.flushp = 0;
+	filter.flushe = sizeof(flushb);
+
+	for (;;) {
+		if (rtnl_wilddump_request(&rth, do_ipv6, RTM_GETROUTE) < 0) {
+			perror("Cannot send dump request");
+			return -2;
+		}
+		filter.flushed = 0;
+		if (rtnl_dump_filter(&rth, filter_fn, stdout) < 0) {
+			fprintf(stderr, "Flush terminated\n");
+			return -2;
+		}
+		if (filter.flushed == 0) {
+			if (show_stats) {
+				if (round == 0 &&
+				    (!filter.cloned || do_ipv6 == AF_INET6))
+					printf("Nothing to flush.\n");
+				else
+					printf("*** Flush is complete after %d round%s ***\n",
+					       round, round > 1 ? "s" : "");
+			}
+			fflush(stdout);
+			return 0;
+		}
+		round++;
+		ret = flush_update();
+		if (ret < 0)
+			return ret;
+
+		if (time(0) - start > 30) {
+			printf("\n*** Flush not completed after %ld seconds, %d entries remain ***\n",
+			       (long)(time(0) - start), filter.flushed);
+			return -1;
+		}
+
+		if (show_stats) {
+			printf("\n*** Round %d, deleting %d entries ***\n",
+			       round, filter.flushed);
+			fflush(stdout);
+		}
+	}
+}
+
 static int iproute_list_flush_or_save(int argc, char **argv, int action)
 {
 	int do_ipv6 = preferred_family;
@@ -1480,7 +1542,6 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 	char *od = NULL;
 	unsigned int mark = 0;
 	rtnl_filter_t filter_fn;
-	int ret;
 
 	if (action == IPROUTE_SAVE) {
 		if (save_route_prep())
@@ -1680,62 +1741,8 @@ static int iproute_list_flush_or_save(int argc, char **argv, int action)
 	}
 	filter.mark = mark;
 
-	if (action == IPROUTE_FLUSH) {
-		int round = 0;
-		char flushb[4096-512];
-		time_t start = time(0);
-
-		if (filter.cloned) {
-			if (do_ipv6 != AF_INET6) {
-				iproute_flush_cache();
-				if (show_stats)
-					printf("*** IPv4 routing cache is flushed.\n");
-			}
-			if (do_ipv6 == AF_INET)
-				return 0;
-		}
-
-		filter.flushb = flushb;
-		filter.flushp = 0;
-		filter.flushe = sizeof(flushb);
-
-		for (;;) {
-			if (rtnl_wilddump_request(&rth, do_ipv6, RTM_GETROUTE) < 0) {
-				perror("Cannot send dump request");
-				return -2;
-			}
-			filter.flushed = 0;
-			if (rtnl_dump_filter(&rth, filter_fn, stdout) < 0) {
-				fprintf(stderr, "Flush terminated\n");
-				return -2;
-			}
-			if (filter.flushed == 0) {
-				if (show_stats) {
-					if (round == 0 && (!filter.cloned || do_ipv6 == AF_INET6))
-						printf("Nothing to flush.\n");
-					else
-						printf("*** Flush is complete after %d round%s ***\n", round, round > 1?"s":"");
-				}
-				fflush(stdout);
-				return 0;
-			}
-			round++;
-			ret = flush_update();
-			if (ret < 0)
-				return ret;
-
-			if (time(0) - start > 30) {
-				printf("\n*** Flush not completed after %ld seconds, %d entries remain ***\n",
-				       (long)(time(0) - start), filter.flushed);
-				return -1;
-			}
-
-			if (show_stats) {
-				printf("\n*** Round %d, deleting %d entries ***\n", round, filter.flushed);
-				fflush(stdout);
-			}
-		}
-	}
+	if (action == IPROUTE_FLUSH)
+		return iproute_flush(do_ipv6, filter_fn);
 
 	if (!filter.cloned) {
 		if (rtnl_wilddump_request(&rth, do_ipv6, RTM_GETROUTE) < 0) {
-- 
2.15.1

Re: [PATCH iproute2-next 0/3] route code cleanups

[David Ahern]

On 2/7/18 5:25 PM, Stephen Hemminger wrote:
> Make code in iproute conform more to current coding style
> conventions. Also split flush out into separate function.
> 
> Stephen Hemminger (3):
>   iproute: whitespace fixes
>   iproute: don't do assignment in condition
>   iproute: make flush a separate function
> 
>  ip/iproute.c | 165 ++++++++++++++++++++++++++++++++---------------------------
>  1 file changed, 91 insertions(+), 74 deletions(-)
> 

applied to iproute2-next