Am 19.10.2017 19:27, schrieb Gustavo A. R. Silva:
> Code refactoring in order to make the code easier to read and maintain.
>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> This code was tested by compilation only (GCC 7.2.0 was used).
>
> net/netrom/nr_route.c | 63
> ++++++++++++++++-----------------------------------
> 1 file changed, 20 insertions(+), 43 deletions(-)
>
> diff --git a/net/netrom/nr_route.c b/net/netrom/nr_route.c
> index fc9cadc..1e5165f 100644
> --- a/net/netrom/nr_route.c
> +++ b/net/netrom/nr_route.c
> @@ -80,6 +80,23 @@ 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 inline void re_sort_routes(struct nr_node *nr_node, int ix_x, int
> ix_y)
> +{
> + struct nr_route nr_route;
> +
> + if (nr_node->routes[ix_y].quality > nr_node->routes[ix_x].quality) {
> + if (nr_node->which == ix_x)
> + nr_node->which = ix_y;
> + else if (nr_node->which == ix_y)
> + nr_node->which = ix_x;
> +
> + nr_route = nr_node->routes[ix_x];
> + nr_node->routes[ix_x] = nr_node->routes[ix_y];
> + nr_node->routes[ix_y] = nr_route;
> + }
> +}
> +
Good idea, a bit of nit picking ..
does ix_ has a special meaning ? otherwise x,y would be sufficient.
>From the code below i can see: y=x+1 Perhaps that can be used.
kernel.h has a swap() macro. so you can
swap(nr_node->routes[x],nr_node->routes[y]);
hope that helps,
re,
wh
> /*
> * Add a new route to a node, and in the process add the node and the
> * neighbour if it is new.
> @@ -90,7 +107,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 +267,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;
> }
