* [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
@ 2013-10-29 11:01 Ying Xue
2013-10-29 11:13 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Ying Xue @ 2013-10-29 11:01 UTC (permalink / raw)
To: davem, maloy; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
The message dispatching part of tipc_recv_msg() is wrapped layers of
while/if/if/switch, causing out-of-control indentation and does not
look very good. We reduce two indentation levels by separating the
message dispatching from the blocks that checks link state and
sequence numbers, allowing longer function and arg names to be
consistently indented without wrapping. This is a cosmetic change
that does not alter the operation of TIPC in any way.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/link.c | 147 +++++++++++++++++++++++++++----------------------------
1 file changed, 72 insertions(+), 75 deletions(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index e8153f6..1bfee8c 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1593,97 +1593,94 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
/* Now (finally!) process the incoming message */
protocol_check:
- if (likely(link_working_working(l_ptr))) {
- if (likely(seq_no == mod(l_ptr->next_in_no))) {
- l_ptr->next_in_no++;
- if (unlikely(l_ptr->oldest_deferred_in))
- head = link_insert_deferred_queue(l_ptr,
- head);
-deliver:
- if (likely(msg_isdata(msg))) {
- tipc_node_unlock(n_ptr);
- tipc_port_recv_msg(buf);
- continue;
- }
- switch (msg_user(msg)) {
- int ret;
- case MSG_BUNDLER:
- l_ptr->stats.recv_bundles++;
- l_ptr->stats.recv_bundled +=
- msg_msgcnt(msg);
- tipc_node_unlock(n_ptr);
- tipc_link_recv_bundle(buf);
- continue;
- case NAME_DISTRIBUTOR:
- n_ptr->bclink.recv_permitted = true;
- tipc_node_unlock(n_ptr);
- tipc_named_recv(buf);
- continue;
- case BCAST_PROTOCOL:
- tipc_link_recv_sync(n_ptr, buf);
- tipc_node_unlock(n_ptr);
- continue;
- case CONN_MANAGER:
- tipc_node_unlock(n_ptr);
- tipc_port_recv_proto_msg(buf);
- continue;
- case MSG_FRAGMENTER:
- l_ptr->stats.recv_fragments++;
- ret = tipc_link_recv_fragment(
- &l_ptr->defragm_buf,
- &buf, &msg);
- if (ret == 1) {
- l_ptr->stats.recv_fragmented++;
- goto deliver;
- }
- if (ret == -1)
- l_ptr->next_in_no--;
- break;
- case CHANGEOVER_PROTOCOL:
- type = msg_type(msg);
- if (link_recv_changeover_msg(&l_ptr,
- &buf)) {
- msg = buf_msg(buf);
- seq_no = msg_seqno(msg);
- if (type == ORIGINAL_MSG)
- goto deliver;
- goto protocol_check;
- }
- break;
- default:
- kfree_skb(buf);
- buf = NULL;
- break;
- }
+ if (unlikely(!link_working_working(l_ptr))) {
+ if (msg_user(msg) == LINK_PROTOCOL) {
+ link_recv_proto_msg(l_ptr, buf);
+ head = link_insert_deferred_queue(l_ptr, head);
+ tipc_node_unlock(n_ptr);
+ continue;
+ }
+
+ /* Traffic message. Conditionally activate link */
+ link_state_event(l_ptr, TRAFFIC_MSG_EVT);
+
+ if (link_working_working(l_ptr)) {
+ /* Re-insert buffer in front of queue */
+ buf->next = head;
+ head = buf;
tipc_node_unlock(n_ptr);
- tipc_net_route_msg(buf);
continue;
}
+ tipc_node_unlock(n_ptr);
+ goto cont;
+ }
+
+ /* Link is now in state WORKING_WORKING */
+ if (unlikely(seq_no != mod(l_ptr->next_in_no))) {
link_handle_out_of_seq_msg(l_ptr, buf);
head = link_insert_deferred_queue(l_ptr, head);
tipc_node_unlock(n_ptr);
continue;
}
-
- /* Link is not in state WORKING_WORKING */
- if (msg_user(msg) == LINK_PROTOCOL) {
- link_recv_proto_msg(l_ptr, buf);
+ l_ptr->next_in_no++;
+ if (unlikely(l_ptr->oldest_deferred_in))
head = link_insert_deferred_queue(l_ptr, head);
+deliver:
+ if (likely(msg_isdata(msg))) {
tipc_node_unlock(n_ptr);
+ tipc_port_recv_msg(buf);
continue;
}
-
- /* Traffic message. Conditionally activate link */
- link_state_event(l_ptr, TRAFFIC_MSG_EVT);
-
- if (link_working_working(l_ptr)) {
- /* Re-insert buffer in front of queue */
- buf->next = head;
- head = buf;
+ switch (msg_user(msg)) {
+ int ret;
+ case MSG_BUNDLER:
+ l_ptr->stats.recv_bundles++;
+ l_ptr->stats.recv_bundled += msg_msgcnt(msg);
+ tipc_node_unlock(n_ptr);
+ tipc_link_recv_bundle(buf);
+ continue;
+ case NAME_DISTRIBUTOR:
+ n_ptr->bclink.recv_permitted = true;
+ tipc_node_unlock(n_ptr);
+ tipc_named_recv(buf);
+ continue;
+ case BCAST_PROTOCOL:
+ tipc_link_recv_sync(n_ptr, buf);
+ tipc_node_unlock(n_ptr);
+ continue;
+ case CONN_MANAGER:
tipc_node_unlock(n_ptr);
+ tipc_port_recv_proto_msg(buf);
continue;
+ case MSG_FRAGMENTER:
+ l_ptr->stats.recv_fragments++;
+ ret = tipc_link_recv_fragment(&l_ptr->defragm_buf,
+ &buf, &msg);
+ if (ret == 1) {
+ l_ptr->stats.recv_fragmented++;
+ goto deliver;
+ }
+ if (ret == -1)
+ l_ptr->next_in_no--;
+ break;
+ case CHANGEOVER_PROTOCOL:
+ type = msg_type(msg);
+ if (link_recv_changeover_msg(&l_ptr, &buf)) {
+ msg = buf_msg(buf);
+ seq_no = msg_seqno(msg);
+ if (type == ORIGINAL_MSG)
+ goto deliver;
+ goto protocol_check;
+ }
+ break;
+ default:
+ kfree_skb(buf);
+ buf = NULL;
+ break;
}
tipc_node_unlock(n_ptr);
+ tipc_net_route_msg(buf);
+ continue;
cont:
kfree_skb(buf);
}
--
1.7.9.5
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
2013-10-29 11:01 [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine Ying Xue
@ 2013-10-29 11:13 ` David Laight
2013-10-29 12:07 ` Erik Hugne
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2013-10-29 11:13 UTC (permalink / raw)
To: Ying Xue, davem, maloy
Cc: Paul.Gortmaker, jon.maloy, erik.hugne, tipc-discussion, netdev
> The message dispatching part of tipc_recv_msg() is wrapped layers of
> while/if/if/switch, causing out-of-control indentation and does not
> look very good. We reduce two indentation levels by separating the
> message dispatching from the blocks that checks link state and
> sequence numbers, allowing longer function and arg names to be
> consistently indented without wrapping. This is a cosmetic change
> that does not alter the operation of TIPC in any way.
...
> + tipc_node_unlock(n_ptr);
> + goto cont;
> + }
...
> continue;
> cont:
> kfree_skb(buf);
> }
I can only see one 'goto cont', an explicit kfree_skb() and
continue would be clearer.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
2013-10-29 11:13 ` David Laight
@ 2013-10-29 12:07 ` Erik Hugne
2013-10-29 12:40 ` Andreas Bofjäll
0 siblings, 1 reply; 6+ messages in thread
From: Erik Hugne @ 2013-10-29 12:07 UTC (permalink / raw)
To: David Laight; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion, davem
On Tue, Oct 29, 2013 at 11:13:09AM +0000, David Laight wrote:
> I can only see one 'goto cont', an explicit kfree_skb() and
> continue would be clearer.
There's actually a few more 'goto cont' in the early error checks
in this function causes it to bail early. Those are not visible
in the patchdiff however.
//E
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
2013-10-29 12:07 ` Erik Hugne
@ 2013-10-29 12:40 ` Andreas Bofjäll
2013-10-29 12:42 ` [tipc-discussion] " David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Bofjäll @ 2013-10-29 12:40 UTC (permalink / raw)
To: Erik Hugne, David Laight
Cc: jon.maloy, netdev, tipc-discussion, davem, Paul.Gortmaker
On 10/29/2013 01:07 PM, Erik Hugne wrote:
> On Tue, Oct 29, 2013 at 11:13:09AM +0000, David Laight wrote:
>> I can only see one 'goto cont', an explicit kfree_skb() and
>> continue would be clearer.
>
> There's actually a few more 'goto cont' in the early error checks
> in this function causes it to bail early. Those are not visible
> in the patchdiff however.
If you wanted to make this a bit simpler, you could make two labels and
move the calls to tipc_node_unlock(n_ptr). The blocks on lines 1545,
1558 and 1614 (in the patched code) could be replaced by a simple "goto
unlock" this way.
--- link.c-old 2013-10-29 13:34:35.804926348 +0100
+++ link.c 2013-10-29 13:39:23.991842809 +0100
@@ -1541,10 +1541,8 @@
/* Locate unicast link endpoint that should handle message */
l_ptr = n_ptr->links[b_ptr->identity];
- if (unlikely(!l_ptr)) {
- tipc_node_unlock(n_ptr);
- goto cont;
- }
+ if (unlikely(!l_ptr))
+ goto unlock;
/* Verify that communication with node is currently allowed */
if ((n_ptr->block_setup & WAIT_PEER_DOWN) &&
@@ -1554,10 +1552,8 @@
!msg_redundant_link(msg))
n_ptr->block_setup &= ~WAIT_PEER_DOWN;
- if (n_ptr->block_setup) {
- tipc_node_unlock(n_ptr);
- goto cont;
- }
+ if (n_ptr->block_setup)
+ goto unlock;
/* Validate message sequence number info */
seq_no = msg_seqno(msg);
@@ -1611,8 +1607,7 @@
tipc_node_unlock(n_ptr);
continue;
}
- tipc_node_unlock(n_ptr);
- goto cont;
+ goto unlock;
}
/* Link is now in state WORKING_WORKING */
@@ -1681,6 +1676,9 @@
tipc_node_unlock(n_ptr);
tipc_net_route_msg(buf);
continue;
+
+unlock:
+ tipc_node_unlock(n_ptr);
cont:
kfree_skb(buf);
}
/Andreas
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [tipc-discussion] [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
2013-10-29 12:40 ` Andreas Bofjäll
@ 2013-10-29 12:42 ` David Laight
2013-10-30 1:34 ` Ying Xue
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2013-10-29 12:42 UTC (permalink / raw)
To: Andreas Bofjäll, Erik Hugne
Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion, davem
> continue;
> +
> +unlock:
> + tipc_node_unlock(n_ptr);
> cont:
> kfree_skb(buf);
> }
Might be better to call the labels 'unlock_discard' and 'discard'.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
2013-10-29 12:42 ` [tipc-discussion] " David Laight
@ 2013-10-30 1:34 ` Ying Xue
0 siblings, 0 replies; 6+ messages in thread
From: Ying Xue @ 2013-10-30 1:34 UTC (permalink / raw)
To: David Laight, Andreas Bofjäll, Erik Hugne
Cc: jon.maloy, netdev, tipc-discussion, davem, Paul.Gortmaker
On 10/29/2013 08:42 PM, David Laight wrote:
>> continue;
>> +
>> +unlock:
>> + tipc_node_unlock(n_ptr);
>> cont:
>> kfree_skb(buf);
>> }
>
> Might be better to call the labels 'unlock_discard' and 'discard'.
>
Thanks for your valuable suggestions. I will submit next version soon.
Regards,
Ying
> David
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-30 1:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-29 11:01 [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine Ying Xue
2013-10-29 11:13 ` David Laight
2013-10-29 12:07 ` Erik Hugne
2013-10-29 12:40 ` Andreas Bofjäll
2013-10-29 12:42 ` [tipc-discussion] " David Laight
2013-10-30 1:34 ` Ying Xue
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).