A number of reworks went into rxrpc_send_call_packet() recently, which
introduced another warning when built with -Wmaybe-uninitialized:

In file included from ../net/rxrpc/output.c:20:0:
net/rxrpc/output.c: In function 'rxrpc_send_call_packet':
net/rxrpc/ar-internal.h:1187:27: error: 'top' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
net/rxrpc/output.c:103:24: note: 'top' was declared here
net/rxrpc/output.c:225:25: error: 'hard_ack' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]

This is a false positive, but it's also an indication that the function is
getting complex enough that the compiler cannot figure out what it does.

This splits out a rxrpc_send_ack_packet() function for one part of
it, making it more understandable by both humans and the compiler
and avoiding the warning.

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
Unfortunately, this is a larger rework that I was hoping for, and
I have only build tested it, so please review carefully, or just
discard it and treat it as feedback to the original patch.
---
 net/rxrpc/output.c | 164 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 88 insertions(+), 76 deletions(-)

diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index cf43a715685e..821af21c7159 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -90,6 +90,86 @@ static size_t rxrpc_fill_out_ack(struct rxrpc_call *call,
        return top - hard_ack + 3;
 }
 
+static int rxrpc_send_ack_packet(struct rxrpc_call *call,
+                                struct rxrpc_connection *conn,
+                                struct msghdr *msg,
+                                struct rxrpc_pkt_buffer *pkt,
+                                struct kvec iov[2])
+{
+       bool ping;
+       size_t len, n;
+       rxrpc_serial_t serial;
+       rxrpc_seq_t hard_ack, top;
+       int ioc, ret;
+
+       spin_lock_bh(&call->lock);
+       if (!call->ackr_reason) {
+               spin_unlock_bh(&call->lock);
+               ret = 0;
+               goto out;
+       }
+       ping = (call->ackr_reason == RXRPC_ACK_PING);
+       n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top);
+       call->ackr_reason = 0;
+
+       spin_unlock_bh(&call->lock);
+
+       pkt->whdr.flags |= RXRPC_SLOW_START_OK;
+
+       iov[0].iov_len += sizeof(pkt->ack) + n;
+       iov[1].iov_base = &pkt->ackinfo;
+       iov[1].iov_len  = sizeof(pkt->ackinfo);
+       len = sizeof(pkt->whdr) + sizeof(pkt->ack) + n + sizeof(pkt->ackinfo);
+       ioc = 2;
+       serial = atomic_inc_return(&conn->serial);
+       pkt->whdr.serial = htonl(serial);
+
+       trace_rxrpc_tx_ack(call, serial,
+                          ntohl(pkt->ack.firstPacket),
+                          ntohl(pkt->ack.serial),
+                          pkt->ack.reason, pkt->ack.nAcks);
+
+       if (ping) {
+               call->ackr_ping = serial;
+               smp_wmb();
+               /* We need to stick a time in before we send the packet in case
+                * the reply gets back before kernel_sendmsg() completes - but
+                * asking UDP to send the packet can take a relatively long
+                * time, so we update the time after, on the assumption that
+                * the packet transmission is more likely to happen towards the
+                * end of the kernel_sendmsg() call.
+                */
+               call->ackr_ping_time = ktime_get_real();
+               set_bit(RXRPC_CALL_PINGING, &call->flags);
+               trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial);
+       }
+       ret = kernel_sendmsg(conn->params.local->socket,
+                            msg, iov, ioc, len);
+       if (ping)
+               call->ackr_ping_time = ktime_get_real();
+
+       if (call->state < RXRPC_CALL_COMPLETE) {
+               if (ret < 0) {
+                       clear_bit(RXRPC_CALL_PINGING, &call->flags);
+                       rxrpc_propose_ACK(call, pkt->ack.reason,
+                                         ntohs(pkt->ack.maxSkew),
+                                         ntohl(pkt->ack.serial),
+                                         true, true,
+                                         rxrpc_propose_ack_retry_tx);
+               } else {
+                       spin_lock_bh(&call->lock);
+                       if (after(hard_ack, call->ackr_consumed))
+                               call->ackr_consumed = hard_ack;
+                       if (after(top, call->ackr_seen))
+                               call->ackr_seen = top;
+                       spin_unlock_bh(&call->lock);
+               }
+       }
+out:
+       return ret;
+}
+
+
 /*
  * Send an ACK or ABORT call packet.
  */
@@ -99,11 +179,9 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 type)
        struct rxrpc_pkt_buffer *pkt;
        struct msghdr msg;
        struct kvec iov[2];
-       rxrpc_serial_t serial;
-       rxrpc_seq_t hard_ack, top;
-       size_t len, n;
-       bool ping = false;
+       size_t len;
        int ioc, ret;
+       rxrpc_serial_t serial;
        u32 abort_code;
 
        _enter("%u,%s", call->debug_id, rxrpc_pkts[type]);
@@ -140,96 +218,30 @@ int rxrpc_send_call_packet(struct rxrpc_call *call, u8 
type)
 
        iov[0].iov_base = pkt;
        iov[0].iov_len  = sizeof(pkt->whdr);
-       len = sizeof(pkt->whdr);
 
        switch (type) {
        case RXRPC_PACKET_TYPE_ACK:
-               spin_lock_bh(&call->lock);
-               if (!call->ackr_reason) {
-                       spin_unlock_bh(&call->lock);
-                       ret = 0;
-                       goto out;
-               }
-               ping = (call->ackr_reason == RXRPC_ACK_PING);
-               n = rxrpc_fill_out_ack(call, pkt, &hard_ack, &top);
-               call->ackr_reason = 0;
-
-               spin_unlock_bh(&call->lock);
-
-
-               pkt->whdr.flags |= RXRPC_SLOW_START_OK;
-
-               iov[0].iov_len += sizeof(pkt->ack) + n;
-               iov[1].iov_base = &pkt->ackinfo;
-               iov[1].iov_len  = sizeof(pkt->ackinfo);
-               len += sizeof(pkt->ack) + n + sizeof(pkt->ackinfo);
-               ioc = 2;
+               ret = rxrpc_send_ack_packet(call, conn, &msg, pkt, iov);
                break;
 
        case RXRPC_PACKET_TYPE_ABORT:
                abort_code = call->abort_code;
                pkt->abort_code = htonl(abort_code);
                iov[0].iov_len += sizeof(pkt->abort_code);
-               len += sizeof(pkt->abort_code);
+               len = sizeof(pkt->whdr) + sizeof(pkt->abort_code);
                ioc = 1;
+               serial = atomic_inc_return(&conn->serial);
+               pkt->whdr.serial = htonl(serial);
+               ret = kernel_sendmsg(conn->params.local->socket,
+                                    &msg, iov, ioc, len);
                break;
 
        default:
                BUG();
                ret = -ENOANO;
-               goto out;
-       }
-
-       serial = atomic_inc_return(&conn->serial);
-       pkt->whdr.serial = htonl(serial);
-       switch (type) {
-       case RXRPC_PACKET_TYPE_ACK:
-               trace_rxrpc_tx_ack(call, serial,
-                                  ntohl(pkt->ack.firstPacket),
-                                  ntohl(pkt->ack.serial),
-                                  pkt->ack.reason, pkt->ack.nAcks);
                break;
        }
 
-       if (ping) {
-               call->ackr_ping = serial;
-               smp_wmb();
-               /* We need to stick a time in before we send the packet in case
-                * the reply gets back before kernel_sendmsg() completes - but
-                * asking UDP to send the packet can take a relatively long
-                * time, so we update the time after, on the assumption that
-                * the packet transmission is more likely to happen towards the
-                * end of the kernel_sendmsg() call.
-                */
-               call->ackr_ping_time = ktime_get_real();
-               set_bit(RXRPC_CALL_PINGING, &call->flags);
-               trace_rxrpc_rtt_tx(call, rxrpc_rtt_tx_ping, serial);
-       }
-       ret = kernel_sendmsg(conn->params.local->socket,
-                            &msg, iov, ioc, len);
-       if (ping)
-               call->ackr_ping_time = ktime_get_real();
-
-       if (type == RXRPC_PACKET_TYPE_ACK &&
-           call->state < RXRPC_CALL_COMPLETE) {
-               if (ret < 0) {
-                       clear_bit(RXRPC_CALL_PINGING, &call->flags);
-                       rxrpc_propose_ACK(call, pkt->ack.reason,
-                                         ntohs(pkt->ack.maxSkew),
-                                         ntohl(pkt->ack.serial),
-                                         true, true,
-                                         rxrpc_propose_ack_retry_tx);
-               } else {
-                       spin_lock_bh(&call->lock);
-                       if (after(hard_ack, call->ackr_consumed))
-                               call->ackr_consumed = hard_ack;
-                       if (after(top, call->ackr_seen))
-                               call->ackr_seen = top;
-                       spin_unlock_bh(&call->lock);
-               }
-       }
-
-out:
        rxrpc_put_connection(conn);
        kfree(pkt);
        return ret;
-- 
2.9.0

Reply via email to