[iobuf] Add iob_disown() and use it where it simplifies code

There are many functions that take ownership of the I/O buffer they
are passed as a parameter.  The caller should not retain a pointer to
the I/O buffer.  Use iob_disown() to automatically nullify the
caller's pointer, e.g.:

    xfer_deliver_iob ( xfer, iob_disown ( iobuf ) );

This will ensure that iobuf is set to NULL for any code after the call
to xfer_deliver_iob().

iob_disown() is currently used only in places where it simplifies the
code, by avoiding an extra line explicitly setting the I/O buffer
pointer to NULL.  It should ideally be used with each call to any
function that takes ownership of an I/O buffer.  (The SSA
optimisations will ensure that use of iob_disown() gets optimised away
in cases where the caller makes no further use of the I/O buffer
pointer anyway.)

If gcc ever introduces an __attribute__((free)), indicating that use
of a function argument after a function call should generate a
warning, then we should use this to identify all applicable function
call sites, and add iob_disown() as necessary.

diff --git a/src/arch/i386/drivers/net/undinet.c b/src/arch/i386/drivers/net/undinet.c
index f9df3438e4a4150e3cc3e4385a05eb30864035fc..7ebfd3c4deb96d84bd039f1d277a4a5079f18592 100644 (file)
--- a/src/arch/i386/drivers/net/undinet.c
+++ b/src/arch/i386/drivers/net/undinet.c
@@ -482,8 +482,7 @@ static void undinet_poll ( struct net_device *netdev ) {
                                         undi_isr.Frame.offset, frag_len );
                        if ( iob_len ( iobuf ) == len ) {
                                /* Whole packet received; deliver it */
-                               netdev_rx ( netdev, iobuf );
-                               iobuf = NULL;
+                               netdev_rx ( netdev, iob_disown ( iobuf ) );
                                /* Etherboot 5.4 fails to return all packets
                                 * under mild load; pretend it retriggered.
                                 */

diff --git a/src/include/gpxe/iobuf.h b/src/include/gpxe/iobuf.h
index db7fa042c534ca04a49030d9566728f5b4c91c4c..6d1a58a44dda4235acf28557b264cf3ef20d1ef7 100644 (file)
--- a/src/include/gpxe/iobuf.h
+++ b/src/include/gpxe/iobuf.h
@@ -199,6 +199,26 @@ static inline void iob_populate ( struct io_buffer *iobuf,
        iobuf->end = ( data + max_len );
 }
 
+/**
+ * Disown an I/O buffer
+ *
+ * @v iobuf    I/O buffer
+ *
+ * There are many functions that take ownership of the I/O buffer they
+ * are passed as a parameter.  The caller should not retain a pointer
+ * to the I/O buffer.  Use iob_disown() to automatically nullify the
+ * caller's pointer, e.g.:
+ *
+ *     xfer_deliver_iob ( xfer, iob_disown ( iobuf ) );
+ *
+ * This will ensure that iobuf is set to NULL for any code after the
+ * call to xfer_deliver_iob().
+ */
+#define iob_disown( iobuf ) ( {                                \
+       struct io_buffer *__iobuf = (iobuf);            \
+       (iobuf) = NULL;                                 \
+       __iobuf; } )
+
 extern struct io_buffer * __malloc alloc_iob ( size_t len );
 extern void free_iob ( struct io_buffer *iobuf );
 extern void iob_pad ( struct io_buffer *iobuf, size_t min_len );

diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c
index 491d2a80246ca12d6dc7bbd46917d6db120ce74f..771b9174b877265b200b7604399691be57c1d171 100644 (file)
--- a/src/interface/efi/efi_snp.c
+++ b/src/interface/efi/efi_snp.c
@@ -602,10 +602,9 @@ efi_snp_transmit ( EFI_SIMPLE_NETWORK_PROTOCOL *snp,
        }
 
        /* Transmit packet */
-       if ( ( rc = netdev_tx ( snpdev->netdev, iobuf ) ) != 0 ) {
+       if ( ( rc = netdev_tx ( snpdev->netdev, iob_disown ( iobuf ) ) ) != 0){
                DBGC ( snpdev, "SNPDEV %p TX could not transmit: %s\n",
                       snpdev, strerror ( rc ) );
-               iobuf = NULL;
                efirc = RC_TO_EFIRC ( rc );
                goto err_tx;
        }

diff --git a/src/net/arp.c b/src/net/arp.c
index 011d4fefb15cf8f947ba1fd3bfebd004c33f8c5f..ba9ebf48872074ca639835dbf282917980947b43 100644 (file)
--- a/src/net/arp.c
+++ b/src/net/arp.c
@@ -265,8 +265,8 @@ static int arp_rx ( struct io_buffer *iobuf, struct net_device *netdev,
        memcpy ( arp_sender_ha ( arphdr ), netdev->ll_addr, arphdr->ar_hln );
 
        /* Send reply */
-       net_tx ( iobuf, netdev, &arp_protocol, arp_target_ha (arphdr ) );
-       iobuf = NULL;
+       net_tx ( iob_disown ( iobuf ), netdev, &arp_protocol,
+                arp_target_ha ( arphdr ) );
 
  done:
        free_iob ( iobuf );

diff --git a/src/net/tcp/http.c b/src/net/tcp/http.c
index 4dc1ab73aacae62bdb3410f58b9120587e7e0fc2..46b5077ad70790df73ee50cab5b7972b6380f3db 100644 (file)
--- a/src/net/tcp/http.c
+++ b/src/net/tcp/http.c
@@ -340,8 +340,7 @@ static int http_socket_deliver_iob ( struct xfer_interface *socket,
                        /* Once we're into the data phase, just fill
                         * the data buffer
                         */
-                       rc = http_rx_data ( http, iobuf );
-                       iobuf = NULL;
+                       rc = http_rx_data ( http, iob_disown ( iobuf ) );
                        goto done;
                case HTTP_RX_RESPONSE:
                case HTTP_RX_HEADER:

diff --git a/src/net/udp.c b/src/net/udp.c
index 63c2d9ee3c491d28e3d21387745797938745f897..57edb9c658275cf57127c6a5fad0ce1f0238e315 100644 (file)
--- a/src/net/udp.c
+++ b/src/net/udp.c
@@ -328,8 +328,7 @@ static int udp_rx ( struct io_buffer *iobuf, struct sockaddr_tcpip *st_src,
        memset ( &meta, 0, sizeof ( meta ) );
        meta.src = ( struct sockaddr * ) st_src;
        meta.dest = ( struct sockaddr * ) st_dest;
-       rc = xfer_deliver_iob_meta ( &udp->xfer, iobuf, &meta );
-       iobuf = NULL;
+       rc = xfer_deliver_iob_meta ( &udp->xfer, iob_disown ( iobuf ), &meta );
 
  done:
        free_iob ( iobuf );

diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index 26c501727e01b8449506c868d3cd8e22b6fcda0c..6ebf60217f79291de1bf0d516bcd062f8f4589eb 100644 (file)
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -976,9 +976,8 @@ static int dhcp_tx ( struct dhcp_session *dhcp ) {
 
        /* Transmit the packet */
        iob_put ( iobuf, dhcppkt.len );
-       rc = xfer_deliver_iob_meta ( &dhcp->xfer, iobuf, &meta );
-       iobuf = NULL;
-       if ( rc != 0 ) {
+       if ( ( rc = xfer_deliver_iob_meta ( &dhcp->xfer, iob_disown ( iobuf ),
+                                           &meta ) ) != 0 ) {
                DBGC ( dhcp, "DHCP %p could not transmit UDP packet: %s\n",
                       dhcp, strerror ( rc ) );
                goto done;

diff --git a/src/net/udp/tftp.c b/src/net/udp/tftp.c
index 13734b0f52a53357f428af2b138a8e093e6bce52..ec6b1b4045b933638651ac1608f7a7911a57ab95 100644 (file)
--- a/src/net/udp/tftp.c
+++ b/src/net/udp/tftp.c
@@ -763,9 +763,8 @@ static int tftp_rx_data ( struct tftp_request *tftp,
        memset ( &meta, 0, sizeof ( meta ) );
        meta.whence = SEEK_SET;
        meta.offset = offset;
-       rc = xfer_deliver_iob_meta ( &tftp->xfer, iobuf, &meta );
-       iobuf = NULL;
-       if ( rc != 0 ) {
+       if ( ( rc = xfer_deliver_iob_meta ( &tftp->xfer, iob_disown ( iobuf ),
+                                           &meta ) ) != 0 ) {
                DBGC ( tftp, "TFTP %p could not deliver data: %s\n",
                       tftp, strerror ( rc ) );
                goto done;
@@ -887,8 +886,7 @@ static int tftp_rx ( struct tftp_request *tftp,
                rc = tftp_rx_oack ( tftp, iobuf->data, len );
                break;
        case htons ( TFTP_DATA ):
-               rc = tftp_rx_data ( tftp, iobuf );
-               iobuf = NULL;
+               rc = tftp_rx_data ( tftp, iob_disown ( iobuf ) );
                break;
        case htons ( TFTP_ERROR ):
                rc = tftp_rx_error ( tftp, iobuf->data, len );