From: walter harms <wharms@bfs.de> To: "Gustavo A. R. Silva" <garsilva@embeddedor.com> Cc: Ralf Baechle <ralf@linux-mips.org>, "David S. Miller" <davem@davemloft.net>, linux-hams@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH 2/2] net: netrom: refactor code in nr_add_node Date: Fri, 20 Oct 2017 10:57:10 +0200 [thread overview] Message-ID: <59E9BA66.4000707@bfs.de> (raw) In-Reply-To: <20171019172746.GA15332@embeddedor.com> 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 <garsilva@embeddedor.com> > --- > 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; > }
WARNING: multiple messages have this Message-ID (diff)
From: walter harms <wharms@bfs.de> To: "Gustavo A. R. Silva" <garsilva@embeddedor.com> Cc: Ralf Baechle <ralf@linux-mips.org>, "David S. Miller" <davem@davemloft.net>, linux-hams@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH 2/2] net: netrom: refactor code in nr_add_node Date: Fri, 20 Oct 2017 10:57:10 +0200 [thread overview] Message-ID: <59E9BA66.4000707@bfs.de> (raw) In-Reply-To: <20171019172746.GA15332@embeddedor.com> 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 <garsilva@embeddedor.com> > --- > 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; > }
next prev parent reply other threads:[~2017-10-20 8:57 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-19 17:21 [PATCH 1/2] net: netrom: mark expected switch fall-throughs Gustavo A. R. Silva 2017-10-19 17:27 ` [PATCH 2/2] net: netrom: refactor code in nr_add_node Gustavo A. R. Silva 2017-10-20 8:57 ` walter harms [this message] 2017-10-20 8:57 ` walter harms 2017-10-20 16:06 ` Gustavo A. R. Silva 2017-10-20 16:54 ` walter harms 2017-10-20 23:09 ` Kevin Dawson 2017-10-23 0:41 ` Gustavo A. R. Silva 2017-10-23 1:08 ` [PATCH v2 " Gustavo A. R. Silva 2017-10-23 1:18 ` David Miller 2017-10-23 1:39 ` Gustavo A. R. Silva 2017-10-27 5:50 ` [PATCH v3 0/2] refactor code and mark expected switch fall-throughs Gustavo A. R. Silva 2017-10-27 5:51 ` [PATCH v3 1/2] net: netrom: nr_route: refactor code in nr_add_node Gustavo A. R. Silva 2017-10-27 5:51 ` [PATCH v3 2/2] net: netrom: nr_route: mark expected switch fall-throughs Gustavo A. R. Silva 2017-10-27 14:47 ` [PATCH v3 0/2] refactor code and " David Ranch 2017-10-27 19:48 ` Gustavo A. R. Silva 2017-10-28 17:53 ` David Ranch 2017-10-29 1:45 ` David Miller 2017-10-29 4:15 ` David Ranch 2017-11-08 22:02 ` f6bvp 2017-11-08 22:02 ` f6bvp 2017-11-01 11:46 ` David Miller 2017-11-01 17:34 ` Gustavo A. R. Silva 2017-10-23 16:19 ` Fwd: [PATCH v2 2/2] net: netrom: refactor code in nr_add_node David Ranch
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=59E9BA66.4000707@bfs.de \ --to=wharms@bfs.de \ --cc=davem@davemloft.net \ --cc=garsilva@embeddedor.com \ --cc=linux-hams@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=ralf@linux-mips.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.