From: Michael Brown Date: Tue, 29 Apr 2025 08:16:41 +0000 (+0100) Subject: [xferbuf] Simplify and generalise data transfer buffers X-Git-Tag: rolling/bin~346 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=837b77293beafa15f8a125a484a0d17453b021f8;p=thirdparty%2Fipxe.git [xferbuf] Simplify and generalise data transfer buffers Since all data transfer buffer contents are now accessible via direct pointer dereferences, remove the unnecessary abstractions for read and write operations and create two new data transfer buffer types: a fixed-size buffer, and a void buffer that records its size but can never receive non-zero lengths of data. These replace the custom data buffer types currently implemented for EFI PXE TFTP downloads and for block device translations. A new operation xferbuf_detach() is required to take ownership of the data accumulated in the data transfer buffer, since we no longer rely on the existence of an independently owned external data pointer for data transfer buffers allocated via umalloc(). Signed-off-by: Michael Brown --- diff --git a/src/core/blocktrans.c b/src/core/blocktrans.c index ba70462f2..362721747 100644 --- a/src/core/blocktrans.c +++ b/src/core/blocktrans.c @@ -38,91 +38,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); #include #include -/** - * Reallocate block device translator data buffer - * - * @v xferbuf Data transfer buffer - * @v len New length (or zero to free buffer) - * @ret rc Return status code - */ -static int blktrans_xferbuf_realloc ( struct xfer_buffer *xferbuf, - size_t len ) { - struct block_translator *blktrans = - container_of ( xferbuf, struct block_translator, xferbuf ); - - /* Record length, if applicable */ - if ( blktrans->buffer ) { - - /* We have a (non-reallocatable) data buffer */ - return -ENOTSUP; - - } else { - - /* Record length (for block device capacity) */ - xferbuf->len = len; - return 0; - } -} - -/** - * Write data to block device translator data buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to copy - * @v len Length of data - */ -static void blktrans_xferbuf_write ( struct xfer_buffer *xferbuf, size_t offset, - const void *data, size_t len ) { - struct block_translator *blktrans = - container_of ( xferbuf, struct block_translator, xferbuf ); - - /* Write data to buffer, if applicable */ - if ( blktrans->buffer ) { - - /* Write data to buffer */ - memcpy ( ( blktrans->buffer + offset ), data, len ); - - } else { - - /* Sanity check */ - assert ( len == 0 ); - } -} - -/** - * Read data from block device translator data buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to read - * @v len Length of data - */ -static void blktrans_xferbuf_read ( struct xfer_buffer *xferbuf, size_t offset, - void *data, size_t len ) { - struct block_translator *blktrans = - container_of ( xferbuf, struct block_translator, xferbuf ); - - /* Read data from buffer, if applicable */ - if ( blktrans->buffer ) { - - /* Read data from buffer */ - memcpy ( data, ( blktrans->buffer + offset ), len ); - - } else { - - /* Sanity check */ - assert ( len == 0 ); - } -} - -/** Block device translator data transfer buffer operations */ -static struct xfer_buffer_operations blktrans_xferbuf_operations = { - .realloc = blktrans_xferbuf_realloc, - .write = blktrans_xferbuf_write, - .read = blktrans_xferbuf_read, -}; - /** * Close block device translator * @@ -233,11 +148,10 @@ int block_translate ( struct interface *block, void *buffer, size_t size ) { ref_init ( &blktrans->refcnt, NULL ); intf_init ( &blktrans->block, &blktrans_block_desc, &blktrans->refcnt ); intf_init ( &blktrans->xfer, &blktrans_xfer_desc, &blktrans->refcnt ); - blktrans->xferbuf.op = &blktrans_xferbuf_operations; - blktrans->buffer = buffer; if ( buffer ) { - blktrans->xferbuf.len = size; + xferbuf_fixed_init ( &blktrans->xferbuf, buffer, size ); } else { + xferbuf_void_init ( &blktrans->xferbuf ); blktrans->blksize = size; } diff --git a/src/core/downloader.c b/src/core/downloader.c index 33737bfac..449761836 100644 --- a/src/core/downloader.c +++ b/src/core/downloader.c @@ -67,6 +67,7 @@ static void downloader_free ( struct refcnt *refcnt ) { struct downloader *downloader = container_of ( refcnt, struct downloader, refcnt ); + xferbuf_free ( &downloader->buffer ); image_put ( downloader->image ); free ( downloader ); } @@ -78,18 +79,21 @@ static void downloader_free ( struct refcnt *refcnt ) { * @v rc Reason for termination */ static void downloader_finished ( struct downloader *downloader, int rc ) { + struct xfer_buffer *buffer = &downloader->buffer; + struct image *image = downloader->image; /* Log download status */ if ( rc == 0 ) { - syslog ( LOG_NOTICE, "Downloaded \"%s\"\n", - downloader->image->name ); + syslog ( LOG_NOTICE, "Downloaded \"%s\"\n", image->name ); } else { syslog ( LOG_ERR, "Download of \"%s\" failed: %s\n", - downloader->image->name, strerror ( rc ) ); + image->name, strerror ( rc ) ); } - /* Update image length */ - downloader->image->len = downloader->buffer.len; + /* Transfer ownership from data transfer buffer to image */ + image->data = buffer->data; + image->len = buffer->len; + xferbuf_detach ( buffer ); /* Shut down interfaces */ intf_shutdown ( &downloader->xfer, rc ); @@ -269,7 +273,7 @@ int create_downloader ( struct interface *job, struct image *image ) { intf_init ( &downloader->xfer, &downloader_xfer_desc, &downloader->refcnt ); downloader->image = image_get ( image ); - xferbuf_umalloc_init ( &downloader->buffer, &image->data ); + xferbuf_umalloc_init ( &downloader->buffer ); /* Instantiate child objects and attach to our interfaces */ if ( ( rc = xfer_open_uri ( &downloader->xfer, image->uri ) ) != 0 ) diff --git a/src/core/xferbuf.c b/src/core/xferbuf.c index 1c08f8bc3..d93526577 100644 --- a/src/core/xferbuf.c +++ b/src/core/xferbuf.c @@ -50,6 +50,21 @@ static struct profiler xferbuf_write_profiler __profiler = static struct profiler xferbuf_read_profiler __profiler = { .name = "xferbuf.read" }; +/** + * Detach data from data transfer buffer + * + * @v xferbuf Data transfer buffer + * + * The caller assumes responsibility for eventually freeing the data + * previously owned by the data transfer buffer. + */ +void xferbuf_detach ( struct xfer_buffer *xferbuf ) { + + xferbuf->data = NULL; + xferbuf->len = 0; + xferbuf->pos = 0; +} + /** * Free data transfer buffer * @@ -58,8 +73,7 @@ static struct profiler xferbuf_read_profiler __profiler = void xferbuf_free ( struct xfer_buffer *xferbuf ) { xferbuf->op->realloc ( xferbuf, 0 ); - xferbuf->len = 0; - xferbuf->pos = 0; + xferbuf_detach ( xferbuf ); } /** @@ -109,9 +123,13 @@ int xferbuf_write ( struct xfer_buffer *xferbuf, size_t offset, if ( ( rc = xferbuf_ensure_size ( xferbuf, max_len ) ) != 0 ) return rc; + /* Check that buffer is non-void */ + if ( len && ( ! xferbuf->data ) ) + return -ENOTTY; + /* Copy data to buffer */ profile_start ( &xferbuf_write_profiler ); - xferbuf->op->write ( xferbuf, offset, data, len ); + memcpy ( ( xferbuf->data + offset ), data, len ); profile_stop ( &xferbuf_write_profiler ); return 0; @@ -133,9 +151,13 @@ int xferbuf_read ( struct xfer_buffer *xferbuf, size_t offset, ( len > ( xferbuf->len - offset ) ) ) return -ENOENT; + /* Check that buffer is non-void */ + if ( len && ( ! xferbuf->data ) ) + return -ENOTTY; + /* Copy data from buffer */ profile_start ( &xferbuf_read_profiler ); - xferbuf->op->read ( xferbuf, offset, data, len ); + memcpy ( data, ( xferbuf->data + offset ), len ); profile_stop ( &xferbuf_read_profiler ); return 0; @@ -178,7 +200,7 @@ int xferbuf_deliver ( struct xfer_buffer *xferbuf, struct io_buffer *iobuf, } /** - * Reallocate malloc()-based data buffer + * Reallocate malloc()-based data transfer buffer * * @v xferbuf Data transfer buffer * @v len New length (or zero to free buffer) @@ -194,94 +216,76 @@ static int xferbuf_malloc_realloc ( struct xfer_buffer *xferbuf, size_t len ) { return 0; } -/** - * Write data to malloc()-based data buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to copy - * @v len Length of data - */ -static void xferbuf_malloc_write ( struct xfer_buffer *xferbuf, size_t offset, - const void *data, size_t len ) { - - memcpy ( ( xferbuf->data + offset ), data, len ); -} - -/** - * Read data from malloc()-based data buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to read - * @v len Length of data - */ -static void xferbuf_malloc_read ( struct xfer_buffer *xferbuf, size_t offset, - void *data, size_t len ) { - - memcpy ( data, ( xferbuf->data + offset ), len ); -} - /** malloc()-based data buffer operations */ struct xfer_buffer_operations xferbuf_malloc_operations = { .realloc = xferbuf_malloc_realloc, - .write = xferbuf_malloc_write, - .read = xferbuf_malloc_read, }; /** - * Reallocate umalloc()-based data buffer + * Reallocate umalloc()-based data transfer buffer * * @v xferbuf Data transfer buffer * @v len New length (or zero to free buffer) * @ret rc Return status code */ static int xferbuf_umalloc_realloc ( struct xfer_buffer *xferbuf, size_t len ) { - void **udata = xferbuf->data; void *new_udata; - new_udata = urealloc ( *udata, len ); + new_udata = urealloc ( xferbuf->data, len ); if ( ! new_udata ) return -ENOSPC; - *udata = new_udata; + xferbuf->data = new_udata; return 0; } +/** umalloc()-based data buffer operations */ +struct xfer_buffer_operations xferbuf_umalloc_operations = { + .realloc = xferbuf_umalloc_realloc, +}; + /** - * Write data to umalloc()-based data buffer + * Reallocate fixed-size data transfer buffer * * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to copy - * @v len Length of data + * @v len New length (or zero to free buffer) + * @ret rc Return status code */ -static void xferbuf_umalloc_write ( struct xfer_buffer *xferbuf, size_t offset, - const void *data, size_t len ) { - void **udata = xferbuf->data; +static int xferbuf_fixed_realloc ( struct xfer_buffer *xferbuf, size_t len ) { + + /* Refuse to allocate extra space */ + if ( len > xferbuf->len ) { + /* Note that EFI relies upon this error mapping to + * EFI_BUFFER_TOO_SMALL. + */ + return -ERANGE; + } - memcpy ( ( *udata + offset ), data, len ); + return 0; } +/** Fixed-size data buffer operations */ +struct xfer_buffer_operations xferbuf_fixed_operations = { + .realloc = xferbuf_fixed_realloc, +}; + /** - * Read data from umalloc()-based data buffer + * Reallocate void data transfer buffer * * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to read - * @v len Length of data + * @v len New length (or zero to free buffer) + * @ret rc Return status code */ -static void xferbuf_umalloc_read ( struct xfer_buffer *xferbuf, size_t offset, - void *data, size_t len ) { - void **udata = xferbuf->data; +static int xferbuf_void_realloc ( struct xfer_buffer *xferbuf, + size_t len __unused ) { - memcpy ( data, ( *udata + offset ), len ); + /* Succeed without ever allocating data */ + assert ( xferbuf->data == NULL ); + return 0; } -/** umalloc()-based data buffer operations */ -struct xfer_buffer_operations xferbuf_umalloc_operations = { - .realloc = xferbuf_umalloc_realloc, - .write = xferbuf_umalloc_write, - .read = xferbuf_umalloc_read, +/** Void data buffer operations */ +struct xfer_buffer_operations xferbuf_void_operations = { + .realloc = xferbuf_void_realloc, }; /** diff --git a/src/include/ipxe/blocktrans.h b/src/include/ipxe/blocktrans.h index 1167a256e..1eb388854 100644 --- a/src/include/ipxe/blocktrans.h +++ b/src/include/ipxe/blocktrans.h @@ -25,8 +25,6 @@ struct block_translator { /** Data transfer buffer */ struct xfer_buffer xferbuf; - /** Data buffer */ - void *buffer; /** Block size */ size_t blksize; }; diff --git a/src/include/ipxe/xferbuf.h b/src/include/ipxe/xferbuf.h index 04635999d..04fcf2286 100644 --- a/src/include/ipxe/xferbuf.h +++ b/src/include/ipxe/xferbuf.h @@ -35,41 +35,19 @@ struct xfer_buffer_operations { * @ret rc Return status code */ int ( * realloc ) ( struct xfer_buffer *xferbuf, size_t len ); - /** Write data to buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to write - * @v len Length of data - * - * This call is simply a wrapper for the appropriate - * memcpy()-like operation: the caller is responsible for - * ensuring that the write does not exceed the buffer length. - */ - void ( * write ) ( struct xfer_buffer *xferbuf, size_t offset, - const void *data, size_t len ); - /** Read data from buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to read - * @v len Length of data - * - * This call is simply a wrapper for the appropriate - * memcpy()-like operation: the caller is responsible for - * ensuring that the read does not exceed the buffer length. - */ - void ( * read ) ( struct xfer_buffer *xferbuf, size_t offset, - void *data, size_t len ); }; extern struct xfer_buffer_operations xferbuf_malloc_operations; extern struct xfer_buffer_operations xferbuf_umalloc_operations; +extern struct xfer_buffer_operations xferbuf_fixed_operations; +extern struct xfer_buffer_operations xferbuf_void_operations; /** * Initialise malloc()-based data transfer buffer * * @v xferbuf Data transfer buffer + * + * Data will be automatically allocated using malloc(). */ static inline __attribute__ (( always_inline )) void xferbuf_malloc_init ( struct xfer_buffer *xferbuf ) { @@ -80,14 +58,45 @@ xferbuf_malloc_init ( struct xfer_buffer *xferbuf ) { * Initialise umalloc()-based data transfer buffer * * @v xferbuf Data transfer buffer - * @v data User pointer + * + * Data will be automatically allocated using umalloc() (and may + * therefore alter the system memory map). */ static inline __attribute__ (( always_inline )) void -xferbuf_umalloc_init ( struct xfer_buffer *xferbuf, void **data ) { - xferbuf->data = data; +xferbuf_umalloc_init ( struct xfer_buffer *xferbuf ) { xferbuf->op = &xferbuf_umalloc_operations; } +/** + * Initialise fixed-size data transfer buffer + * + * @v xferbuf Data transfer buffer + * @v data Data buffer + * @v len Length of data buffer + * + * Data will be never be automatically allocated. + */ +static inline __attribute__ (( always_inline )) void +xferbuf_fixed_init ( struct xfer_buffer *xferbuf, void *data, size_t len ) { + xferbuf->data = data; + xferbuf->len = len; + xferbuf->op = &xferbuf_fixed_operations; +} + +/** + * Initialise void data transfer buffer + * + * @v xferbuf Data transfer buffer + * + * No data will be allocated, but the length will be recorded. This + * can be used to capture xfer_seek() results. + */ +static inline __attribute__ (( always_inline )) void +xferbuf_void_init ( struct xfer_buffer *xferbuf ) { + xferbuf->op = &xferbuf_void_operations; +} + +extern void xferbuf_detach ( struct xfer_buffer *xferbuf ); extern void xferbuf_free ( struct xfer_buffer *xferbuf ); extern int xferbuf_write ( struct xfer_buffer *xferbuf, size_t offset, const void *data, size_t len ); diff --git a/src/interface/efi/efi_pxe.c b/src/interface/efi/efi_pxe.c index 732ccdcdf..4ba057019 100644 --- a/src/interface/efi/efi_pxe.c +++ b/src/interface/efi/efi_pxe.c @@ -303,48 +303,6 @@ static int efi_pxe_ip_filter ( struct efi_pxe *pxe, EFI_IP_ADDRESS *ip ) { return 0; } -/****************************************************************************** - * - * Data transfer buffer - * - ****************************************************************************** - */ - -/** - * Reallocate PXE data transfer buffer - * - * @v xferbuf Data transfer buffer - * @v len New length (or zero to free buffer) - * @ret rc Return status code - */ -static int efi_pxe_buf_realloc ( struct xfer_buffer *xferbuf __unused, - size_t len __unused ) { - - /* Can never reallocate: return EFI_BUFFER_TOO_SMALL */ - return -ERANGE; -} - -/** - * Write data to PXE data transfer buffer - * - * @v xferbuf Data transfer buffer - * @v offset Starting offset - * @v data Data to copy - * @v len Length of data - */ -static void efi_pxe_buf_write ( struct xfer_buffer *xferbuf, size_t offset, - const void *data, size_t len ) { - - /* Copy data to buffer */ - memcpy ( ( xferbuf->data + offset ), data, len ); -} - -/** PXE data transfer buffer operations */ -static struct xfer_buffer_operations efi_pxe_buf_operations = { - .realloc = efi_pxe_buf_realloc, - .write = efi_pxe_buf_write, -}; - /****************************************************************************** * * (M)TFTP download interface @@ -966,8 +924,7 @@ efi_pxe_mtftp ( EFI_PXE_BASE_CODE_PROTOCOL *base, pxe->blksize = ( ( callback && blksize ) ? *blksize : -1UL ); /* Initialise data transfer buffer */ - pxe->buf.data = data; - pxe->buf.len = *len; + xferbuf_fixed_init ( &pxe->buf, data, *len ); /* Open download */ if ( ( rc = efi_pxe_tftp_open ( pxe, ip, @@ -987,6 +944,7 @@ efi_pxe_mtftp ( EFI_PXE_BASE_CODE_PROTOCOL *base, err_download: efi_pxe_tftp_close ( pxe, rc ); err_open: + xferbuf_fixed_init ( &pxe->buf, NULL, 0 ); efi_snp_release(); err_opcode: return EFIRC ( rc ); @@ -1611,7 +1569,6 @@ int efi_pxe_install ( EFI_HANDLE handle, struct net_device *netdev ) { pxe->base.Mode = &pxe->mode; memcpy ( &pxe->apple, &efi_apple_net_boot_protocol, sizeof ( pxe->apple ) ); - pxe->buf.op = &efi_pxe_buf_operations; intf_init ( &pxe->tftp, &efi_pxe_tftp_desc, &pxe->refcnt ); intf_init ( &pxe->udp, &efi_pxe_udp_desc, &pxe->refcnt ); INIT_LIST_HEAD ( &pxe->queue ); diff --git a/src/net/peermux.c b/src/net/peermux.c index a391ed373..431ca76e0 100644 --- a/src/net/peermux.c +++ b/src/net/peermux.c @@ -129,6 +129,7 @@ static int peermux_info_deliver ( struct peerdist_multiplexer *peermux, * @v rc Reason for close */ static void peermux_info_close ( struct peerdist_multiplexer *peermux, int rc ){ + struct xfer_buffer *buffer = &peermux->buffer; struct peerdist_info *info = &peermux->cache.info; size_t len; @@ -145,8 +146,7 @@ static void peermux_info_close ( struct peerdist_multiplexer *peermux, int rc ){ intf_shutdown ( &peermux->info, rc ); /* Parse content information */ - if ( ( rc = peerdist_info ( info->raw.data, peermux->buffer.len, - info ) ) != 0 ) { + if ( ( rc = peerdist_info ( buffer->data, buffer->len, info ) ) != 0 ) { DBGC ( peermux, "PEERMUX %p could not parse content info: %s\n", peermux, strerror ( rc ) ); goto err; @@ -422,8 +422,7 @@ int peermux_filter ( struct interface *xfer, struct interface *info, intf_init ( &peermux->xfer, &peermux_xfer_desc, &peermux->refcnt ); intf_init ( &peermux->info, &peermux_info_desc, &peermux->refcnt ); peermux->uri = uri_get ( uri ); - xferbuf_umalloc_init ( &peermux->buffer, - &peermux->cache.info.raw.data ); + xferbuf_umalloc_init ( &peermux->buffer ); process_init_stopped ( &peermux->process, &peermux_process_desc, &peermux->refcnt ); INIT_LIST_HEAD ( &peermux->busy );