diff mbox series

[v2,2/2] net: netrom: refactor code in nr_add_node

Message ID 20171023010840.GA32460@embeddedor.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series None | expand

Commit Message

Gustavo A. R. Silva Oct. 23, 2017, 1:08 a.m. UTC 
Code refactoring in order to make it easier to read and maintain.

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only.

Changes in v2:
 Make use of the swap macro and remove inline keyword.

 net/netrom/nr_route.c | 59 ++++++++++++++-------------------------------------
 1 file changed, 16 insertions(+), 43 deletions(-)

Comments

David Miller Oct. 23, 2017, 1:18 a.m. UTC | #1 
From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
Date: Sun, 22 Oct 2017 20:08:40 -0500

> Code refactoring in order to make it easier to read and maintain.
> 
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Gustavo, always when reposting a new version of a patch that is part of
a series you must _always_ repost the entire patch series.

Also, a proper patch series must begine with a "[PATCH 0/2] ..."
header posting explaining at a high level what the patch series
is doing, how it is doing it, and why it is doing it that way.

Thank you.
Gustavo A. R. Silva Oct. 23, 2017, 1:39 a.m. UTC | #2 
Quoting David Miller <davem@davemloft.net>:

> From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
> Date: Sun, 22 Oct 2017 20:08:40 -0500
>
>> Code refactoring in order to make it easier to read and maintain.
>>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>
> Gustavo, always when reposting a new version of a patch that is part of
> a series you must _always_ repost the entire patch series.
>

OK. I got it.

> Also, a proper patch series must begine with a "[PATCH 0/2] ..."
> header posting explaining at a high level what the patch series
> is doing, how it is doing it, and why it is doing it that way.
>

Yeah, in this case I thought there was no need for this as both  
patches are not actually related in terms of functionality. But now  
that I'm writing this, maybe that is precisely the reason why I should  
have posted such header...?

Thanks
--
Gustavo A. R. Silva
diff mbox series

Patch

diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
index fc9cadc..505e142 100644
--- a/net/netrom/nr_route.c
+++ b/net/netrom/nr_route.c
@@ -80,6 +80,19 @@  static struct nr_neigh *nr_neigh_get_dev(ax25_address *callsign,
 
 static void nr_remove_neigh(struct nr_neigh *);
 
+/*      re-sort the routes in quality order.    */
+static void re_sort_routes(struct nr_node *nr_node, int x, int y)
+{
+	if (nr_node->routes[y].quality > nr_node->routes[x].quality) {
+		if (nr_node->which == x)
+			nr_node->which = y;
+		else if (nr_node->which == y)
+			nr_node->which = x;
+
+		swap(nr_node->routes[x], nr_node->routes[y]);
+	}
+}
+
 /*
  *	Add a new route to a node, and in the process add the node and the
  *	neighbour if it is new.
@@ -90,7 +103,6 @@  static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
 {
 	struct nr_node  *nr_node;
 	struct nr_neigh *nr_neigh;
-	struct nr_route nr_route;
 	int i, found;
 	struct net_device *odev;
 
@@ -251,50 +263,11 @@  static int __must_check nr_add_node(ax25_address *nr, const char *mnemonic,
 	/* Now re-sort the routes in quality order */
 	switch (nr_node->count) {
 	case 3:
-		if (nr_node->routes[1].quality > nr_node->routes[0].quality) {
-			switch (nr_node->which) {
-			case 0:
-				nr_node->which = 1;
-				break;
-			case 1:
-				nr_node->which = 0;
-				break;
-			}
-			nr_route           = nr_node->routes[0];
-			nr_node->routes[0] = nr_node->routes[1];
-			nr_node->routes[1] = nr_route;
-		}
-		if (nr_node->routes[2].quality > nr_node->routes[1].quality) {
-			switch (nr_node->which) {
-			case 1:  nr_node->which = 2;
-				break;
-
-			case 2:  nr_node->which = 1;
-				break;
-
-			default:
-				break;
-			}
-			nr_route           = nr_node->routes[1];
-			nr_node->routes[1] = nr_node->routes[2];
-			nr_node->routes[2] = nr_route;
-		}
+		re_sort_routes(nr_node, 0, 1);
+		re_sort_routes(nr_node, 1, 2);
 		/* fall through */
 	case 2:
-		if (nr_node->routes[1].quality > nr_node->routes[0].quality) {
-			switch (nr_node->which) {
-			case 0:  nr_node->which = 1;
-				break;
-
-			case 1:  nr_node->which = 0;
-				break;
-
-			default: break;
-			}
-			nr_route           = nr_node->routes[0];
-			nr_node->routes[0] = nr_node->routes[1];
-			nr_node->routes[1] = nr_route;
-			}
+		re_sort_routes(nr_node, 0, 1);
 	case 1:
 		break;
 	}