From dbe84c5aad583a4194666d2d925a5cda53852631 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Sun, 1 Feb 2009 18:02:28 +0000 Subject: [PATCH] [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. --- src/arch/i386/drivers/net/undinet.c | 3 +-- src/include/gpxe/iobuf.h | 20 ++++++++++++++++++++ src/interface/efi/efi_snp.c | 3 +-- src/net/arp.c | 4 ++-- src/net/tcp/http.c | 3 +-- src/net/udp.c | 3 +-- src/net/udp/dhcp.c | 5 ++--- src/net/udp/tftp.c | 8 +++----- 8 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/arch/i386/drivers/net/undinet.c b/src/arch/i386/drivers/net/undinet.c index f9df3438..7ebfd3c4 100644 --- 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 db7fa042..6d1a58a4 100644 --- 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 491d2a80..771b9174 100644 --- 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 011d4fef..ba9ebf48 100644 --- 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 4dc1ab73..46b5077a 100644 --- 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 63c2d9ee..57edb9c6 100644 --- 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 26c50172..6ebf6021 100644 --- 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 13734b0f..ec6b1b40 100644 --- 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 );