Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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.
  • Loading branch information
Michael Brown committed Feb 1, 2009
1 parent 0171098 commit dbe84c5
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 18 deletions.
3 changes: 1 addition & 2 deletions src/arch/i386/drivers/net/undinet.c
Expand Up @@ -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.
*/
Expand Down
20 changes: 20 additions & 0 deletions src/include/gpxe/iobuf.h
Expand Up @@ -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 );
Expand Down
3 changes: 1 addition & 2 deletions src/interface/efi/efi_snp.c
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/net/arp.c
Expand Up @@ -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 );
Expand Down
3 changes: 1 addition & 2 deletions src/net/tcp/http.c
Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions src/net/udp.c
Expand Up @@ -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 );
Expand Down
5 changes: 2 additions & 3 deletions src/net/udp/dhcp.c
Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions src/net/udp/tftp.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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 );
Expand Down

0 comments on commit dbe84c5

Please sign in to comment.