diff mbox

[net-next,3/7] packet: rollover prepare: single return in packet_rcv_has_room

Message ID 1430936837-22655-4-git-send-email-willemb@google.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Willem de Bruijn May 6, 2015, 6:27 p.m. UTC 
From: Willem de Bruijn <willemb@google.com>

Convert

  if (x) {
    A;
    return y;
  }
  B;
  return y;

Into

  if (x) {
    A;
  } else {
    B;
  }

  return y;

No other changes.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/packet/af_packet.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

David Laight May 7, 2015, 1:49 p.m. UTC | #1 
From: Willem de Bruijn
> Sent: 06 May 2015 19:27
> Convert
> 
>   if (x) {
>     A;
>     return y;
>   }
>   B;
>   return y;
> 
> Into
> 
>   if (x) {
>     A;
>   } else {
>     B;
>   }
> 
>   return y;
> 
> No other changes.

...

IMHO there is little benefit in the 'single return from function' style.
In general it just makes scan-reading of code more difficult (once you
see a 'return' you know then is no more relevant code) and hits the RH margin.

	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
Willem de Bruijn May 7, 2015, 4:05 p.m. UTC | #2 
> IMHO there is little benefit in the 'single return from function' style.
> In general it just makes scan-reading of code more difficult (once you
> see a 'return' you know then is no more relevant code) and hits the RH margin.

Subsequent patches add code to the footer of the function. I should
have been more clear about that in the commit message. The change
is indeed not very useful on its own. But at the end of the patch set,
this is the footer:

      has_room = ret == ROOM_NORMAL;
      if (po->pressure == has_room)
              xchg(&po->pressure, !has_room);

      return ret;

which otherwise has to be duplicated.
--
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
diff mbox

Patch

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index f8ec909..8275056 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1239,20 +1239,21 @@  static bool packet_rcv_has_room(struct packet_sock *po, struct sk_buff *skb)
 	struct sock *sk = &po->sk;
 	bool has_room;
 
-	if (po->prot_hook.func != tpacket_rcv)
-		return (atomic_read(&sk->sk_rmem_alloc) + skb->truesize)
-			<= sk->sk_rcvbuf;
-
-	spin_lock(&sk->sk_receive_queue.lock);
-	if (po->tp_version == TPACKET_V3)
-		has_room = prb_lookup_block(po, &po->rx_ring,
-					    po->rx_ring.prb_bdqc.kactive_blk_num,
-					    TP_STATUS_KERNEL);
-	else
-		has_room = packet_lookup_frame(po, &po->rx_ring,
-					       po->rx_ring.head,
-					       TP_STATUS_KERNEL);
-	spin_unlock(&sk->sk_receive_queue.lock);
+	if (po->prot_hook.func != tpacket_rcv) {
+		has_room = (atomic_read(&sk->sk_rmem_alloc) + skb->truesize)
+			   <= sk->sk_rcvbuf;
+	} else {
+		spin_lock(&sk->sk_receive_queue.lock);
+		if (po->tp_version == TPACKET_V3)
+			has_room = prb_lookup_block(po, &po->rx_ring,
+						    po->rx_ring.prb_bdqc.kactive_blk_num,
+						    TP_STATUS_KERNEL);
+		else
+			has_room = packet_lookup_frame(po, &po->rx_ring,
+						       po->rx_ring.head,
+						       TP_STATUS_KERNEL);
+		spin_unlock(&sk->sk_receive_queue.lock);
+	}
 
 	return has_room;
 }