[PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Simon Glass]

The membuff implementation curently has no tests. It also assumes that
head and tail can never correspond unless the buffer is empty.

This series provides a compile-time flag to support a 'full' flag. It
also adds some tests of the main routines.

The data structure is also renamed to membuf which fits better with
U-Boot.

There may be some cases in the code which could be optimised a little,
but the implementation is functional.


Simon Glass (8):
  membuff: Rename functions to have membuf_ prefix
  membuff: Rename the files to membuf
  membuf: Rename struct
  membuf: Include stdbool
  membuf: Correct implementation of membuf_dispose()
  membuf: Add some tests
  membuf: Minor code-style improvements
  membuf: Support a flag for being full

 MAINTAINERS                       |   7 +
 arch/sandbox/include/asm/serial.h |   2 +-
 boot/bootmeth_extlinux.c          |   8 +-
 common/console.c                  |  32 ++--
 drivers/serial/sandbox.c          |  10 +-
 drivers/usb/emul/sandbox_keyb.c   |   8 +-
 include/asm-generic/global_data.h |   6 +-
 include/{membuff.h => membuf.h}   |  98 ++++++------
 lib/Makefile                      |   2 +-
 lib/{membuff.c => membuf.c}       | 134 ++++++++++------
 test/lib/Makefile                 |   1 +
 test/lib/membuf.c                 | 244 ++++++++++++++++++++++++++++++
 12 files changed, 427 insertions(+), 125 deletions(-)
 rename include/{membuff.h => membuf.h} (67%)
 rename lib/{membuff.c => membuf.c} (69%)
 create mode 100644 test/lib/membuf.c

-- 
2.34.1

[PATCH 1/8] membuff: Rename functions to have membuf_ prefix

[Simon Glass]

The double 'f' is not necessary and is a bit annoying as elsewhere in
U-Boot we use 'buf'. Rename all the functions before it is used more
widely.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootmeth_extlinux.c        |  6 +--
 common/console.c                | 32 ++++++-------
 drivers/serial/sandbox.c        | 10 ++--
 drivers/usb/emul/sandbox_keyb.c |  6 +--
 include/membuff.h               | 82 ++++++++++++++++-----------------
 lib/membuff.c                   | 66 +++++++++++++-------------
 6 files changed, 101 insertions(+), 101 deletions(-)

diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
index be8fbf4df63..35854c1e188 100644
--- a/boot/bootmeth_extlinux.c
+++ b/boot/bootmeth_extlinux.c
@@ -112,9 +112,9 @@ static int extlinux_fill_info(struct bootflow *bflow)
 	int len;
 
 	log_debug("parsing bflow file size %x\n", bflow->size);
-	membuff_init(&mb, bflow->buf, bflow->size);
-	membuff_putraw(&mb, bflow->size, true, &data);
-	while (len = membuff_readline(&mb, line, sizeof(line) - 1, ' ', true), len) {
+	membuf_init(&mb, bflow->buf, bflow->size);
+	membuf_putraw(&mb, bflow->size, true, &data);
+	while (len = membuf_readline(&mb, line, sizeof(line) - 1, ' ', true), len) {
 		char *tok, *p = line;
 
 		tok = strsep(&p, " ");
diff --git a/common/console.c b/common/console.c
index 22224701e45..fa4017a188a 100644
--- a/common/console.c
+++ b/common/console.c
@@ -101,7 +101,7 @@ static void console_record_putc(const char c)
 	if (!(gd->flags & GD_FLG_RECORD))
 		return;
 	if  (gd->console_out.start &&
-	     !membuff_putbyte((struct membuff *)&gd->console_out, c))
+	     !membuf_putbyte((struct membuff *)&gd->console_out, c))
 		gd->flags |= GD_FLG_RECORD_OVF;
 }
 
@@ -112,7 +112,7 @@ static void console_record_puts(const char *s)
 	if  (gd->console_out.start) {
 		int len = strlen(s);
 
-		if (membuff_put((struct membuff *)&gd->console_out, s, len) !=
+		if (membuf_put((struct membuff *)&gd->console_out, s, len) !=
 		    len)
 			gd->flags |= GD_FLG_RECORD_OVF;
 	}
@@ -125,7 +125,7 @@ static int console_record_getc(void)
 	if (!gd->console_in.start)
 		return -1;
 
-	return membuff_getbyte((struct membuff *)&gd->console_in);
+	return membuf_getbyte((struct membuff *)&gd->console_in);
 }
 
 static int console_record_tstc(void)
@@ -133,7 +133,7 @@ static int console_record_tstc(void)
 	if (!(gd->flags & GD_FLG_RECORD))
 		return 0;
 	if (gd->console_in.start) {
-		if (membuff_peekbyte((struct membuff *)&gd->console_in) != -1)
+		if (membuf_peekbyte((struct membuff *)&gd->console_in) != -1)
 			return 1;
 	}
 	return 0;
@@ -814,14 +814,14 @@ int console_record_init(void)
 {
 	int ret;
 
-	ret = membuff_new((struct membuff *)&gd->console_out,
-			  gd->flags & GD_FLG_RELOC ?
-				  CONFIG_CONSOLE_RECORD_OUT_SIZE :
-				  CONFIG_CONSOLE_RECORD_OUT_SIZE_F);
+	ret = membuf_new((struct membuff *)&gd->console_out,
+			 gd->flags & GD_FLG_RELOC ?
+				CONFIG_CONSOLE_RECORD_OUT_SIZE :
+				CONFIG_CONSOLE_RECORD_OUT_SIZE_F);
 	if (ret)
 		return ret;
-	ret = membuff_new((struct membuff *)&gd->console_in,
-			  CONFIG_CONSOLE_RECORD_IN_SIZE);
+	ret = membuf_new((struct membuff *)&gd->console_in,
+			 CONFIG_CONSOLE_RECORD_IN_SIZE);
 
 	/* Start recording from the beginning */
 	gd->flags |= GD_FLG_RECORD;
@@ -831,8 +831,8 @@ int console_record_init(void)
 
 void console_record_reset(void)
 {
-	membuff_purge((struct membuff *)&gd->console_out);
-	membuff_purge((struct membuff *)&gd->console_in);
+	membuf_purge((struct membuff *)&gd->console_out);
+	membuf_purge((struct membuff *)&gd->console_in);
 	gd->flags &= ~GD_FLG_RECORD_OVF;
 }
 
@@ -851,23 +851,23 @@ int console_record_readline(char *str, int maxlen)
 	if (console_record_isempty())
 		return -ENOENT;
 
-	return membuff_readline((struct membuff *)&gd->console_out, str,
+	return membuf_readline((struct membuff *)&gd->console_out, str,
 				maxlen, '\0', false);
 }
 
 int console_record_avail(void)
 {
-	return membuff_avail((struct membuff *)&gd->console_out);
+	return membuf_avail((struct membuff *)&gd->console_out);
 }
 
 bool console_record_isempty(void)
 {
-	return membuff_isempty((struct membuff *)&gd->console_out);
+	return membuf_isempty((struct membuff *)&gd->console_out);
 }
 
 int console_in_puts(const char *str)
 {
-	return membuff_put((struct membuff *)&gd->console_in, str, strlen(str));
+	return membuf_put((struct membuff *)&gd->console_in, str, strlen(str));
 }
 
 #endif
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index 77a1558db68..cc0491bc3c8 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -63,7 +63,7 @@ static int sandbox_serial_probe(struct udevice *dev)
 
 	if (state->term_raw != STATE_TERM_RAW)
 		disable_ctrlc(1);
-	membuff_init(&priv->buf, priv->serial_buf, sizeof(priv->serial_buf));
+	membuf_init(&priv->buf, priv->serial_buf, sizeof(priv->serial_buf));
 
 	return 0;
 }
@@ -138,15 +138,15 @@ static int sandbox_serial_pending(struct udevice *dev, bool input)
 		return 0;
 
 	os_usleep(100);
-	avail = membuff_putraw(&priv->buf, 100, false, &data);
+	avail = membuf_putraw(&priv->buf, 100, false, &data);
 	if (!avail)
 		return 1;	/* buffer full */
 
 	count = os_read(0, data, avail);
 	if (count > 0)
-		membuff_putraw(&priv->buf, count, true, &data);
+		membuf_putraw(&priv->buf, count, true, &data);
 
-	return membuff_avail(&priv->buf);
+	return membuf_avail(&priv->buf);
 }
 
 static int sandbox_serial_getc(struct udevice *dev)
@@ -156,7 +156,7 @@ static int sandbox_serial_getc(struct udevice *dev)
 	if (!sandbox_serial_pending(dev, true))
 		return -EAGAIN;	/* buffer empty */
 
-	return membuff_getbyte(&priv->buf);
+	return membuf_getbyte(&priv->buf);
 }
 
 #ifdef CONFIG_DEBUG_UART_SANDBOX
diff --git a/drivers/usb/emul/sandbox_keyb.c b/drivers/usb/emul/sandbox_keyb.c
index db769883ba3..756fef64a36 100644
--- a/drivers/usb/emul/sandbox_keyb.c
+++ b/drivers/usb/emul/sandbox_keyb.c
@@ -167,7 +167,7 @@ int sandbox_usb_keyb_add_string(struct udevice *dev,
 	struct sandbox_keyb_priv *priv = dev_get_priv(dev);
 	int ret;
 
-	ret = membuff_put(&priv->in, scancode, USB_KBD_BOOT_REPORT_SIZE);
+	ret = membuf_put(&priv->in, scancode, USB_KBD_BOOT_REPORT_SIZE);
 	if (ret != USB_KBD_BOOT_REPORT_SIZE)
 		return -ENOSPC;
 
@@ -194,7 +194,7 @@ static int sandbox_keyb_interrupt(struct udevice *dev, struct usb_device *udev,
 	if (length < USB_KBD_BOOT_REPORT_SIZE)
 		return 0;
 
-	membuff_get(&priv->in, buffer, USB_KBD_BOOT_REPORT_SIZE);
+	membuf_get(&priv->in, buffer, USB_KBD_BOOT_REPORT_SIZE);
 
 	return 0;
 }
@@ -220,7 +220,7 @@ static int sandbox_keyb_probe(struct udevice *dev)
 	struct sandbox_keyb_priv *priv = dev_get_priv(dev);
 
 	/* Provide an 80 character keyboard buffer */
-	return membuff_new(&priv->in, 80 * USB_KBD_BOOT_REPORT_SIZE);
+	return membuf_new(&priv->in, 80 * USB_KBD_BOOT_REPORT_SIZE);
 }
 
 static const struct dm_usb_ops sandbox_usb_keyb_ops = {
diff --git a/include/membuff.h b/include/membuff.h
index 4eba626ce1c..c7370e1c072 100644
--- a/include/membuff.h
+++ b/include/membuff.h
@@ -6,8 +6,8 @@
  * Copyright (c) 1992 Simon Glass
  */
 
-#ifndef _MEMBUFF_H
-#define _MEMBUFF_H
+#ifndef _membuf_H
+#define _membuf_H
 
 /**
  * @struct membuff: holds the state of a membuff - it is used for input and
@@ -37,16 +37,16 @@ struct membuff {
 };
 
 /**
- * membuff_purge() - reset a membuff to the empty state
+ * membuf_purge() - reset a membuff to the empty state
  *
  * Initialise head and tail pointers so that the membuff becomes empty.
  *
  * @mb: membuff to purge
  */
-void membuff_purge(struct membuff *mb);
+void membuf_purge(struct membuff *mb);
 
 /**
- * membuff_putraw() - find out where bytes can be written
+ * membuf_putraw() - find out where bytes can be written
  *
  * Work out where in the membuff some data could be written. Return a pointer
  * to the address and the number of bytes which can be written there. If
@@ -64,10 +64,10 @@ void membuff_purge(struct membuff *mb);
  * @data: the address data can be written to
  * Return: number of bytes which can be written
  */
-int membuff_putraw(struct membuff *mb, int maxlen, bool update, char **data);
+int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data);
 
 /**
- * membuff_getraw() - find and return a pointer to available bytes
+ * membuf_getraw() - find and return a pointer to available bytes
  *
  * Returns a pointer to any valid input data in the given membuff and
  * optionally marks it as read. Note that not all input data may not be
@@ -82,37 +82,37 @@ int membuff_putraw(struct membuff *mb, int maxlen, bool update, char **data);
  * @data: returns address of data in input membuff
  * Return: the number of bytes available at *@data
  */
-int membuff_getraw(struct membuff *mb, int maxlen, bool update, char **data);
+int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data);
 
 /**
- * membuff_putbyte() - Writes a byte to a membuff
+ * membuf_putbyte() - Writes a byte to a membuff
  *
  * @mb: membuff to adjust
  * @ch: byte to write
  * Return: true on success, false if membuff is full
  */
-bool membuff_putbyte(struct membuff *mb, int ch);
+bool membuf_putbyte(struct membuff *mb, int ch);
 
 /**
  * @mb: membuff to adjust
- * membuff_getbyte() - Read a byte from the membuff
+ * membuf_getbyte() - Read a byte from the membuff
  * Return: the byte read, or -1 if the membuff is empty
  */
-int membuff_getbyte(struct membuff *mb);
+int membuf_getbyte(struct membuff *mb);
 
 /**
- * membuff_peekbyte() - check the next available byte
+ * membuf_peekbyte() - check the next available byte
  *
- * Return the next byte which membuff_getbyte() would return, without
+ * Return the next byte which membuf_getbyte() would return, without
  * removing it from the membuff.
  *
  * @mb: membuff to adjust
  * Return: the byte peeked, or -1 if the membuff is empty
  */
-int membuff_peekbyte(struct membuff *mb);
+int membuf_peekbyte(struct membuff *mb);
 
 /**
- * membuff_get() - get data from a membuff
+ * membuf_get() - get data from a membuff
  *
  * Copies any available data (up to @maxlen bytes) to @buff and removes it
  * from the membuff.
@@ -122,10 +122,10 @@ int membuff_peekbyte(struct membuff *mb);
  * @maxlen: maximum number of bytes to read
  * Return: the number of bytes read
  */
-int membuff_get(struct membuff *mb, char *buff, int maxlen);
+int membuf_get(struct membuff *mb, char *buff, int maxlen);
 
 /**
- * membuff_put() - write data to a membuff
+ * membuf_put() - write data to a membuff
  *
  * Writes some data to a membuff. Returns the number of bytes added. If this
  * is less than @lnehgt, then the membuff got full
@@ -135,36 +135,36 @@ int membuff_get(struct membuff *mb, char *buff, int maxlen);
  * @length: number of bytes to write from 'data'
  * Return: the number of bytes added
  */
-int membuff_put(struct membuff *mb, const char *buff, int length);
+int membuf_put(struct membuff *mb, const char *buff, int length);
 
 /**
- * membuff_isempty() - check if a membuff is empty
+ * membuf_isempty() - check if a membuff is empty
  *
  * @mb: membuff to check
  * Return: true if empty, else false
  */
-bool membuff_isempty(struct membuff *mb);
+bool membuf_isempty(struct membuff *mb);
 
 /**
- * membuff_avail() - check available data in a membuff
+ * membuf_avail() - check available data in a membuff
  *
  * @mb: membuff to check
  * Return: number of bytes of data available
  */
-int membuff_avail(struct membuff *mb);
+int membuf_avail(struct membuff *mb);
 
 /**
- * membuff_size() - get the size of a membuff
+ * membuf_size() - get the size of a membuff
  *
  * Note that a membuff can only old data up to one byte less than its size.
  *
  * @mb: membuff to check
  * Return: total size
  */
-int membuff_size(struct membuff *mb);
+int membuf_size(struct membuff *mb);
 
 /**
- * membuff_makecontig() - adjust all membuff data to be contiguous
+ * membuf_makecontig() - adjust all membuff data to be contiguous
  *
  * This places all data in a membuff into a single contiguous lump, if
  * possible
@@ -172,18 +172,18 @@ int membuff_size(struct membuff *mb);
  * @mb: membuff to adjust
  * Return: true on success
  */
-bool membuff_makecontig(struct membuff *mb);
+bool membuf_makecontig(struct membuff *mb);
 
 /**
- * membuff_free() - find the number of bytes that can be written to a membuff
+ * membuf_free() - find the number of bytes that can be written to a membuff
  *
  * @mb: membuff to check
  * Return: returns the number of bytes free in a membuff
  */
-int membuff_free(struct membuff *mb);
+int membuf_free(struct membuff *mb);
 
 /**
- * membuff_readline() - read a line of text from a membuff
+ * membuf_readline() - read a line of text from a membuff
  *
  * Reads a line of text of up to 'maxlen' characters from a membuff and puts
  * it in @str. Any character less than @minch is assumed to be the end of
@@ -196,10 +196,10 @@ int membuff_free(struct membuff *mb);
  * Return: number of bytes read (including terminator) if a line has been
  *	   read, 0 if nothing was there or line didn't fit when must_fit is set
  */
-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);
+int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);
 
 /**
- * membuff_extend_by() - expand a membuff
+ * membuf_extend_by() - expand a membuff
  *
  * Extends a membuff by the given number of bytes
  *
@@ -209,38 +209,38 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool
  * Return: 0 if the expand succeeded, -ENOMEM if not enough memory, -E2BIG
  * if the the size would exceed @max
  */
-int membuff_extend_by(struct membuff *mb, int by, int max);
+int membuf_extend_by(struct membuff *mb, int by, int max);
 
 /**
- * membuff_init() - set up a new membuff using an existing membuff
+ * membuf_init() - set up a new membuff using an existing membuff
  *
  * @mb: membuff to set up
  * @buff: Address of buffer
  * @size: Size of buffer
  */
-void membuff_init(struct membuff *mb, char *buff, int size);
+void membuf_init(struct membuff *mb, char *buff, int size);
 
 /**
- * membuff_uninit() - clear a membuff so it can no longer be used
+ * membuf_uninit() - clear a membuff so it can no longer be used
  *
  * @mb: membuff to uninit
  */
-void membuff_uninit(struct membuff *mb);
+void membuf_uninit(struct membuff *mb);
 
 /**
- * membuff_new() - create a new membuff
+ * membuf_new() - create a new membuff
  *
  * @mb: membuff to init
  * @size: size of membuff to create
  * Return: 0 if OK, -ENOMEM if out of memory
  */
-int membuff_new(struct membuff *mb, int size);
+int membuf_new(struct membuff *mb, int size);
 
 /**
- * membuff_dispose() - free memory allocated to a membuff and uninit it
+ * membuf_dispose() - free memory allocated to a membuff and uninit it
  *
  * @mb: membuff to dispose
  */
-void membuff_dispose(struct membuff *mb);
+void membuf_dispose(struct membuff *mb);
 
 #endif
diff --git a/lib/membuff.c b/lib/membuff.c
index b242a38ff1c..435d12b8a3c 100644
--- a/lib/membuff.c
+++ b/lib/membuff.c
@@ -11,15 +11,15 @@
 #include <malloc.h>
 #include "membuff.h"
 
-void membuff_purge(struct membuff *mb)
+void membuf_purge(struct membuff *mb)
 {
 	/* set mb->head and mb->tail so the buffers look empty */
 	mb->head = mb->start;
 	mb->tail = mb->start;
 }
 
-static int membuff_putrawflex(struct membuff *mb, int maxlen, bool update,
-			      char ***data, int *offsetp)
+static int membuf_putrawflex(struct membuff *mb, int maxlen, bool update,
+			     char ***data, int *offsetp)
 {
 	int len;
 
@@ -72,30 +72,30 @@ static int membuff_putrawflex(struct membuff *mb, int maxlen, bool update,
 	return len;
 }
 
-int membuff_putraw(struct membuff *mb, int maxlen, bool update, char **data)
+int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data)
 {
 	char **datap;
 	int offset;
 	int size;
 
-	size = membuff_putrawflex(mb, maxlen, update, &datap, &offset);
+	size = membuf_putrawflex(mb, maxlen, update, &datap, &offset);
 	*data = *datap + offset;
 
 	return size;
 }
 
-bool membuff_putbyte(struct membuff *mb, int ch)
+bool membuf_putbyte(struct membuff *mb, int ch)
 {
 	char *data;
 
-	if (membuff_putraw(mb, 1, true, &data) != 1)
+	if (membuf_putraw(mb, 1, true, &data) != 1)
 		return false;
 	*data = ch;
 
 	return true;
 }
 
-int membuff_getraw(struct membuff *mb, int maxlen, bool update, char **data)
+int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data)
 {
 	int len;
 
@@ -146,21 +146,21 @@ int membuff_getraw(struct membuff *mb, int maxlen, bool update, char **data)
 	return len;
 }
 
-int membuff_getbyte(struct membuff *mb)
+int membuf_getbyte(struct membuff *mb)
 {
 	char *data = 0;
 
-	return membuff_getraw(mb, 1, true, &data) != 1 ? -1 : *(uint8_t *)data;
+	return membuf_getraw(mb, 1, true, &data) != 1 ? -1 : *(uint8_t *)data;
 }
 
-int membuff_peekbyte(struct membuff *mb)
+int membuf_peekbyte(struct membuff *mb)
 {
 	char *data = 0;
 
-	return membuff_getraw(mb, 1, false, &data) != 1 ? -1 : *(uint8_t *)data;
+	return membuf_getraw(mb, 1, false, &data) != 1 ? -1 : *(uint8_t *)data;
 }
 
-int membuff_get(struct membuff *mb, char *buff, int maxlen)
+int membuf_get(struct membuff *mb, char *buff, int maxlen)
 {
 	char *data = 0, *buffptr = buff;
 	int len = 1, i;
@@ -171,7 +171,7 @@ int membuff_get(struct membuff *mb, char *buff, int maxlen)
 	 */
 	for (i = 0; len && i < 2; i++) {
 		/* get a pointer to the data available */
-		len = membuff_getraw(mb, maxlen, true, &data);
+		len = membuf_getraw(mb, maxlen, true, &data);
 
 		/* copy it into the buffer */
 		memcpy(buffptr, data, len);
@@ -183,14 +183,14 @@ int membuff_get(struct membuff *mb, char *buff, int maxlen)
 	return buffptr - buff;
 }
 
-int membuff_put(struct membuff *mb, const char *buff, int length)
+int membuf_put(struct membuff *mb, const char *buff, int length)
 {
 	char *data;
 	int towrite, i, written;
 
 	for (i = written = 0; i < 2; i++) {
 		/* ask where some data can be written */
-		towrite = membuff_putraw(mb, length, true, &data);
+		towrite = membuf_putraw(mb, length, true, &data);
 
 		/* and write it, updating the bytes length */
 		memcpy(data, buff, towrite);
@@ -203,12 +203,12 @@ int membuff_put(struct membuff *mb, const char *buff, int length)
 	return written;
 }
 
-bool membuff_isempty(struct membuff *mb)
+bool membuf_isempty(struct membuff *mb)
 {
 	return mb->head == mb->tail;
 }
 
-int membuff_avail(struct membuff *mb)
+int membuf_avail(struct membuff *mb)
 {
 	struct membuff copy;
 	int i, avail;
@@ -219,18 +219,18 @@ int membuff_avail(struct membuff *mb)
 
 	/* now read everything out of the copied buffer */
 	for (i = avail = 0; i < 2; i++)
-		avail += membuff_getraw(&copy, -1, true, &data);
+		avail += membuf_getraw(&copy, -1, true, &data);
 
 	/* and return how much we read */
 	return avail;
 }
 
-int membuff_size(struct membuff *mb)
+int membuf_size(struct membuff *mb)
 {
 	return mb->end - mb->start;
 }
 
-bool membuff_makecontig(struct membuff *mb)
+bool membuf_makecontig(struct membuff *mb)
 {
 	int topsize, botsize;
 
@@ -281,13 +281,13 @@ bool membuff_makecontig(struct membuff *mb)
 	return true;
 }
 
-int membuff_free(struct membuff *mb)
+int membuf_free(struct membuff *mb)
 {
 	return mb->end == mb->start ? 0 :
-			(mb->end - mb->start) - 1 - membuff_avail(mb);
+			(mb->end - mb->start) - 1 - membuf_avail(mb);
 }
 
-int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)
+int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)
 {
 	int len;  /* number of bytes read (!= string length) */
 	char *s, *end;
@@ -322,7 +322,7 @@ int membuff_readline(struct membuff *mb, char *str, int maxlen, int minch, bool
 	return len;
 }
 
-int membuff_extend_by(struct membuff *mb, int by, int max)
+int membuf_extend_by(struct membuff *mb, int by, int max)
 {
 	int oldhead, oldtail;
 	int size, orig;
@@ -358,32 +358,32 @@ int membuff_extend_by(struct membuff *mb, int by, int max)
 	return 0;
 }
 
-void membuff_init(struct membuff *mb, char *buff, int size)
+void membuf_init(struct membuff *mb, char *buff, int size)
 {
 	mb->start = buff;
 	mb->end = mb->start + size;
-	membuff_purge(mb);
+	membuf_purge(mb);
 }
 
-int membuff_new(struct membuff *mb, int size)
+int membuf_new(struct membuff *mb, int size)
 {
 	mb->start = malloc(size);
 	if (!mb->start)
 		return -ENOMEM;
 
-	membuff_init(mb, mb->start, size);
+	membuf_init(mb, mb->start, size);
 	return 0;
 }
 
-void membuff_uninit(struct membuff *mb)
+void membuf_uninit(struct membuff *mb)
 {
 	mb->end = NULL;
 	mb->start = NULL;
-	membuff_purge(mb);
+	membuf_purge(mb);
 }
 
-void membuff_dispose(struct membuff *mb)
+void membuf_dispose(struct membuff *mb)
 {
 	free(&mb->start);
-	membuff_uninit(mb);
+	membuf_uninit(mb);
 }
-- 
2.34.1

Re: [PATCH 1/8] membuff: Rename functions to have membuf_ prefix

[Simon Glass]

The double 'f' is not necessary and is a bit annoying as elsewhere in
U-Boot we use 'buf'. Rename all the functions before it is used more
widely.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 boot/bootmeth_extlinux.c        |  6 +--
 common/console.c                | 32 ++++++-------
 drivers/serial/sandbox.c        | 10 ++--
 drivers/usb/emul/sandbox_keyb.c |  6 +--
 include/membuff.h               | 82 ++++++++++++++++-----------------
 lib/membuff.c                   | 66 +++++++++++++-------------
 6 files changed, 101 insertions(+), 101 deletions(-)

Applied to sjg/master, thanks!

[PATCH 2/8] membuff: Rename the files to membuf

[Simon Glass]

Rename the C and header files to use the membuf basename, to match the
functions.

Add a MAINTAINERS entry while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 MAINTAINERS                       | 7 +++++++
 include/asm-generic/global_data.h | 2 +-
 include/{membuff.h => membuf.h}   | 0
 lib/Makefile                      | 2 +-
 lib/{membuff.c => membuf.c}       | 2 +-
 5 files changed, 10 insertions(+), 3 deletions(-)
 rename include/{membuff.h => membuf.h} (100%)
 rename lib/{membuff.c => membuf.c} (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index eef63530f68..7e12fc82a63 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1242,6 +1242,13 @@ T:	git git://github.com/ARM-software/u-boot.git
 F:	drivers/video/mali_dp.c
 F:	drivers/i2c/i2c-versatile.c
 
+MEMBUF
+M:	Simon Glass <sjg@chromium.org>
+S:	Maintained
+T:	git https://source.denx.de/u-boot/u-boot.git
+F:	include/membuf.h
+F:	lib/membuf.c
+
 MICROBLAZE
 M:	Michal Simek <monstr@monstr.eu>
 S:	Maintained
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 644a0d77873..cc87d0037f0 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -24,7 +24,7 @@
 #include <cyclic.h>
 #include <event_internal.h>
 #include <fdtdec.h>
-#include <membuff.h>
+#include <membuf.h>
 #include <linux/list.h>
 #include <linux/build_bug.h>
 #include <asm-offsets.h>
diff --git a/include/membuff.h b/include/membuf.h
similarity index 100%
rename from include/membuff.h
rename to include/membuf.h
diff --git a/lib/Makefile b/lib/Makefile
index dbcfa87ebd6..57cc4b0bdc8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -124,7 +124,7 @@ obj-y += hang.o
 obj-y += linux_compat.o
 obj-y += linux_string.o
 obj-$(CONFIG_$(PHASE_)LMB) += lmb.o
-obj-y += membuff.o
+obj-y += membuf.o
 obj-$(CONFIG_REGEX) += slre.o
 obj-y += string.o
 obj-y += tables_csum.o
diff --git a/lib/membuff.c b/lib/membuf.c
similarity index 99%
rename from lib/membuff.c
rename to lib/membuf.c
index 435d12b8a3c..5473efa98ca 100644
--- a/lib/membuff.c
+++ b/lib/membuf.c
@@ -9,7 +9,7 @@
 #include <errno.h>
 #include <log.h>
 #include <malloc.h>
-#include "membuff.h"
+#include "membuf.h"
 
 void membuf_purge(struct membuff *mb)
 {
-- 
2.34.1

[PATCH 3/8] membuf: Rename struct

[Simon Glass]

Rename the struct to match the function prefix and filenames.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/include/asm/serial.h |  2 +-
 boot/bootmeth_extlinux.c          |  2 +-
 common/console.c                  | 24 +++++++++---------
 drivers/usb/emul/sandbox_keyb.c   |  2 +-
 include/asm-generic/global_data.h |  4 +--
 include/membuf.h                  | 42 +++++++++++++++----------------
 lib/membuf.c                      | 42 +++++++++++++++----------------
 7 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/arch/sandbox/include/asm/serial.h b/arch/sandbox/include/asm/serial.h
index 16589a1b219..41506341816 100644
--- a/arch/sandbox/include/asm/serial.h
+++ b/arch/sandbox/include/asm/serial.h
@@ -44,7 +44,7 @@ void sandbox_serial_endisable(bool enabled);
  * @buf: holds input characters available to be read by this driver
  */
 struct sandbox_serial_priv {
-	struct membuff buf;
+	struct membuf buf;
 	char serial_buf[16];
 	bool start_of_line;
 };
diff --git a/boot/bootmeth_extlinux.c b/boot/bootmeth_extlinux.c
index 35854c1e188..e193feb180e 100644
--- a/boot/bootmeth_extlinux.c
+++ b/boot/bootmeth_extlinux.c
@@ -106,7 +106,7 @@ static int extlinux_check(struct udevice *dev, struct bootflow_iter *iter)
  */
 static int extlinux_fill_info(struct bootflow *bflow)
 {
-	struct membuff mb;
+	struct membuf mb;
 	char line[200];
 	char *data;
 	int len;
diff --git a/common/console.c b/common/console.c
index fa4017a188a..5a7e9cf422f 100644
--- a/common/console.c
+++ b/common/console.c
@@ -101,7 +101,7 @@ static void console_record_putc(const char c)
 	if (!(gd->flags & GD_FLG_RECORD))
 		return;
 	if  (gd->console_out.start &&
-	     !membuf_putbyte((struct membuff *)&gd->console_out, c))
+	     !membuf_putbyte((struct membuf *)&gd->console_out, c))
 		gd->flags |= GD_FLG_RECORD_OVF;
 }
 
@@ -112,7 +112,7 @@ static void console_record_puts(const char *s)
 	if  (gd->console_out.start) {
 		int len = strlen(s);
 
-		if (membuf_put((struct membuff *)&gd->console_out, s, len) !=
+		if (membuf_put((struct membuf *)&gd->console_out, s, len) !=
 		    len)
 			gd->flags |= GD_FLG_RECORD_OVF;
 	}
@@ -125,7 +125,7 @@ static int console_record_getc(void)
 	if (!gd->console_in.start)
 		return -1;
 
-	return membuf_getbyte((struct membuff *)&gd->console_in);
+	return membuf_getbyte((struct membuf *)&gd->console_in);
 }
 
 static int console_record_tstc(void)
@@ -133,7 +133,7 @@ static int console_record_tstc(void)
 	if (!(gd->flags & GD_FLG_RECORD))
 		return 0;
 	if (gd->console_in.start) {
-		if (membuf_peekbyte((struct membuff *)&gd->console_in) != -1)
+		if (membuf_peekbyte((struct membuf *)&gd->console_in) != -1)
 			return 1;
 	}
 	return 0;
@@ -814,13 +814,13 @@ int console_record_init(void)
 {
 	int ret;
 
-	ret = membuf_new((struct membuff *)&gd->console_out,
+	ret = membuf_new((struct membuf *)&gd->console_out,
 			 gd->flags & GD_FLG_RELOC ?
 				CONFIG_CONSOLE_RECORD_OUT_SIZE :
 				CONFIG_CONSOLE_RECORD_OUT_SIZE_F);
 	if (ret)
 		return ret;
-	ret = membuf_new((struct membuff *)&gd->console_in,
+	ret = membuf_new((struct membuf *)&gd->console_in,
 			 CONFIG_CONSOLE_RECORD_IN_SIZE);
 
 	/* Start recording from the beginning */
@@ -831,8 +831,8 @@ int console_record_init(void)
 
 void console_record_reset(void)
 {
-	membuf_purge((struct membuff *)&gd->console_out);
-	membuf_purge((struct membuff *)&gd->console_in);
+	membuf_purge((struct membuf *)&gd->console_out);
+	membuf_purge((struct membuf *)&gd->console_in);
 	gd->flags &= ~GD_FLG_RECORD_OVF;
 }
 
@@ -851,23 +851,23 @@ int console_record_readline(char *str, int maxlen)
 	if (console_record_isempty())
 		return -ENOENT;
 
-	return membuf_readline((struct membuff *)&gd->console_out, str,
+	return membuf_readline((struct membuf *)&gd->console_out, str,
 				maxlen, '\0', false);
 }
 
 int console_record_avail(void)
 {
-	return membuf_avail((struct membuff *)&gd->console_out);
+	return membuf_avail((struct membuf *)&gd->console_out);
 }
 
 bool console_record_isempty(void)
 {
-	return membuf_isempty((struct membuff *)&gd->console_out);
+	return membuf_isempty((struct membuf *)&gd->console_out);
 }
 
 int console_in_puts(const char *str)
 {
-	return membuf_put((struct membuff *)&gd->console_in, str, strlen(str));
+	return membuf_put((struct membuf *)&gd->console_in, str, strlen(str));
 }
 
 #endif
diff --git a/drivers/usb/emul/sandbox_keyb.c b/drivers/usb/emul/sandbox_keyb.c
index 756fef64a36..5ed8c2c799a 100644
--- a/drivers/usb/emul/sandbox_keyb.c
+++ b/drivers/usb/emul/sandbox_keyb.c
@@ -38,7 +38,7 @@ enum {
  *
  */
 struct sandbox_keyb_priv {
-	struct membuff in;
+	struct membuf in;
 };
 
 struct sandbox_keyb_plat {
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index cc87d0037f0..9f10e3cddd5 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -317,14 +317,14 @@ struct global_data {
 	 *
 	 * This buffer is used to collect output during console recording.
 	 */
-	struct membuff console_out;
+	struct membuf console_out;
 	/**
 	 * @console_in: input buffer for console recording
 	 *
 	 * If console recording is activated, this buffer can be used to
 	 * emulate input.
 	 */
-	struct membuff console_in;
+	struct membuf console_in;
 #endif
 #if CONFIG_IS_ENABLED(VIDEO)
 	/**
diff --git a/include/membuf.h b/include/membuf.h
index c7370e1c072..17616d5577e 100644
--- a/include/membuf.h
+++ b/include/membuf.h
@@ -10,7 +10,7 @@
 #define _membuf_H
 
 /**
- * @struct membuff: holds the state of a membuff - it is used for input and
+ * @struct membuf: holds the state of a membuff - it is used for input and
  * output buffers. The buffer extends from @start to (@start + @size - 1).
  * Data in the buffer extends from @tail to @head: it is written in at
  * @head and read out from @tail. The membuff is empty when @head == @tail
@@ -29,7 +29,7 @@
  *		^		^
  *		head		tail
  */
-struct membuff {
+struct membuf {
 	char *start;		/** the start of the buffer */
 	char *end;		/** the end of the buffer (start + length) */
 	char *head;		/** current buffer head */
@@ -43,7 +43,7 @@ struct membuff {
  *
  * @mb: membuff to purge
  */
-void membuf_purge(struct membuff *mb);
+void membuf_purge(struct membuf *mb);
 
 /**
  * membuf_putraw() - find out where bytes can be written
@@ -64,7 +64,7 @@ void membuf_purge(struct membuff *mb);
  * @data: the address data can be written to
  * Return: number of bytes which can be written
  */
-int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data);
+int membuf_putraw(struct membuf *mb, int maxlen, bool update, char **data);
 
 /**
  * membuf_getraw() - find and return a pointer to available bytes
@@ -82,7 +82,7 @@ int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data);
  * @data: returns address of data in input membuff
  * Return: the number of bytes available at *@data
  */
-int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data);
+int membuf_getraw(struct membuf *mb, int maxlen, bool update, char **data);
 
 /**
  * membuf_putbyte() - Writes a byte to a membuff
@@ -91,14 +91,14 @@ int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data);
  * @ch: byte to write
  * Return: true on success, false if membuff is full
  */
-bool membuf_putbyte(struct membuff *mb, int ch);
+bool membuf_putbyte(struct membuf *mb, int ch);
 
 /**
  * @mb: membuff to adjust
  * membuf_getbyte() - Read a byte from the membuff
  * Return: the byte read, or -1 if the membuff is empty
  */
-int membuf_getbyte(struct membuff *mb);
+int membuf_getbyte(struct membuf *mb);
 
 /**
  * membuf_peekbyte() - check the next available byte
@@ -109,7 +109,7 @@ int membuf_getbyte(struct membuff *mb);
  * @mb: membuff to adjust
  * Return: the byte peeked, or -1 if the membuff is empty
  */
-int membuf_peekbyte(struct membuff *mb);
+int membuf_peekbyte(struct membuf *mb);
 
 /**
  * membuf_get() - get data from a membuff
@@ -122,7 +122,7 @@ int membuf_peekbyte(struct membuff *mb);
  * @maxlen: maximum number of bytes to read
  * Return: the number of bytes read
  */
-int membuf_get(struct membuff *mb, char *buff, int maxlen);
+int membuf_get(struct membuf *mb, char *buff, int maxlen);
 
 /**
  * membuf_put() - write data to a membuff
@@ -135,7 +135,7 @@ int membuf_get(struct membuff *mb, char *buff, int maxlen);
  * @length: number of bytes to write from 'data'
  * Return: the number of bytes added
  */
-int membuf_put(struct membuff *mb, const char *buff, int length);
+int membuf_put(struct membuf *mb, const char *buff, int length);
 
 /**
  * membuf_isempty() - check if a membuff is empty
@@ -143,7 +143,7 @@ int membuf_put(struct membuff *mb, const char *buff, int length);
  * @mb: membuff to check
  * Return: true if empty, else false
  */
-bool membuf_isempty(struct membuff *mb);
+bool membuf_isempty(struct membuf *mb);
 
 /**
  * membuf_avail() - check available data in a membuff
@@ -151,7 +151,7 @@ bool membuf_isempty(struct membuff *mb);
  * @mb: membuff to check
  * Return: number of bytes of data available
  */
-int membuf_avail(struct membuff *mb);
+int membuf_avail(struct membuf *mb);
 
 /**
  * membuf_size() - get the size of a membuff
@@ -161,7 +161,7 @@ int membuf_avail(struct membuff *mb);
  * @mb: membuff to check
  * Return: total size
  */
-int membuf_size(struct membuff *mb);
+int membuf_size(struct membuf *mb);
 
 /**
  * membuf_makecontig() - adjust all membuff data to be contiguous
@@ -172,7 +172,7 @@ int membuf_size(struct membuff *mb);
  * @mb: membuff to adjust
  * Return: true on success
  */
-bool membuf_makecontig(struct membuff *mb);
+bool membuf_makecontig(struct membuf *mb);
 
 /**
  * membuf_free() - find the number of bytes that can be written to a membuff
@@ -180,7 +180,7 @@ bool membuf_makecontig(struct membuff *mb);
  * @mb: membuff to check
  * Return: returns the number of bytes free in a membuff
  */
-int membuf_free(struct membuff *mb);
+int membuf_free(struct membuf *mb);
 
 /**
  * membuf_readline() - read a line of text from a membuff
@@ -196,7 +196,7 @@ int membuf_free(struct membuff *mb);
  * Return: number of bytes read (including terminator) if a line has been
  *	   read, 0 if nothing was there or line didn't fit when must_fit is set
  */
-int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit);
+int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch, bool must_fit);
 
 /**
  * membuf_extend_by() - expand a membuff
@@ -209,7 +209,7 @@ int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool m
  * Return: 0 if the expand succeeded, -ENOMEM if not enough memory, -E2BIG
  * if the the size would exceed @max
  */
-int membuf_extend_by(struct membuff *mb, int by, int max);
+int membuf_extend_by(struct membuf *mb, int by, int max);
 
 /**
  * membuf_init() - set up a new membuff using an existing membuff
@@ -218,14 +218,14 @@ int membuf_extend_by(struct membuff *mb, int by, int max);
  * @buff: Address of buffer
  * @size: Size of buffer
  */
-void membuf_init(struct membuff *mb, char *buff, int size);
+void membuf_init(struct membuf *mb, char *buff, int size);
 
 /**
  * membuf_uninit() - clear a membuff so it can no longer be used
  *
  * @mb: membuff to uninit
  */
-void membuf_uninit(struct membuff *mb);
+void membuf_uninit(struct membuf *mb);
 
 /**
  * membuf_new() - create a new membuff
@@ -234,13 +234,13 @@ void membuf_uninit(struct membuff *mb);
  * @size: size of membuff to create
  * Return: 0 if OK, -ENOMEM if out of memory
  */
-int membuf_new(struct membuff *mb, int size);
+int membuf_new(struct membuf *mb, int size);
 
 /**
  * membuf_dispose() - free memory allocated to a membuff and uninit it
  *
  * @mb: membuff to dispose
  */
-void membuf_dispose(struct membuff *mb);
+void membuf_dispose(struct membuf *mb);
 
 #endif
diff --git a/lib/membuf.c b/lib/membuf.c
index 5473efa98ca..b13998ccdbd 100644
--- a/lib/membuf.c
+++ b/lib/membuf.c
@@ -11,14 +11,14 @@
 #include <malloc.h>
 #include "membuf.h"
 
-void membuf_purge(struct membuff *mb)
+void membuf_purge(struct membuf *mb)
 {
 	/* set mb->head and mb->tail so the buffers look empty */
 	mb->head = mb->start;
 	mb->tail = mb->start;
 }
 
-static int membuf_putrawflex(struct membuff *mb, int maxlen, bool update,
+static int membuf_putrawflex(struct membuf *mb, int maxlen, bool update,
 			     char ***data, int *offsetp)
 {
 	int len;
@@ -72,7 +72,7 @@ static int membuf_putrawflex(struct membuff *mb, int maxlen, bool update,
 	return len;
 }
 
-int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data)
+int membuf_putraw(struct membuf *mb, int maxlen, bool update, char **data)
 {
 	char **datap;
 	int offset;
@@ -84,7 +84,7 @@ int membuf_putraw(struct membuff *mb, int maxlen, bool update, char **data)
 	return size;
 }
 
-bool membuf_putbyte(struct membuff *mb, int ch)
+bool membuf_putbyte(struct membuf *mb, int ch)
 {
 	char *data;
 
@@ -95,7 +95,7 @@ bool membuf_putbyte(struct membuff *mb, int ch)
 	return true;
 }
 
-int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data)
+int membuf_getraw(struct membuf *mb, int maxlen, bool update, char **data)
 {
 	int len;
 
@@ -146,21 +146,21 @@ int membuf_getraw(struct membuff *mb, int maxlen, bool update, char **data)
 	return len;
 }
 
-int membuf_getbyte(struct membuff *mb)
+int membuf_getbyte(struct membuf *mb)
 {
 	char *data = 0;
 
 	return membuf_getraw(mb, 1, true, &data) != 1 ? -1 : *(uint8_t *)data;
 }
 
-int membuf_peekbyte(struct membuff *mb)
+int membuf_peekbyte(struct membuf *mb)
 {
 	char *data = 0;
 
 	return membuf_getraw(mb, 1, false, &data) != 1 ? -1 : *(uint8_t *)data;
 }
 
-int membuf_get(struct membuff *mb, char *buff, int maxlen)
+int membuf_get(struct membuf *mb, char *buff, int maxlen)
 {
 	char *data = 0, *buffptr = buff;
 	int len = 1, i;
@@ -183,7 +183,7 @@ int membuf_get(struct membuff *mb, char *buff, int maxlen)
 	return buffptr - buff;
 }
 
-int membuf_put(struct membuff *mb, const char *buff, int length)
+int membuf_put(struct membuf *mb, const char *buff, int length)
 {
 	char *data;
 	int towrite, i, written;
@@ -203,14 +203,14 @@ int membuf_put(struct membuff *mb, const char *buff, int length)
 	return written;
 }
 
-bool membuf_isempty(struct membuff *mb)
+bool membuf_isempty(struct membuf *mb)
 {
 	return mb->head == mb->tail;
 }
 
-int membuf_avail(struct membuff *mb)
+int membuf_avail(struct membuf *mb)
 {
-	struct membuff copy;
+	struct membuf copy;
 	int i, avail;
 	char *data = 0;
 
@@ -225,12 +225,12 @@ int membuf_avail(struct membuff *mb)
 	return avail;
 }
 
-int membuf_size(struct membuff *mb)
+int membuf_size(struct membuf *mb)
 {
 	return mb->end - mb->start;
 }
 
-bool membuf_makecontig(struct membuff *mb)
+bool membuf_makecontig(struct membuf *mb)
 {
 	int topsize, botsize;
 
@@ -281,13 +281,13 @@ bool membuf_makecontig(struct membuff *mb)
 	return true;
 }
 
-int membuf_free(struct membuff *mb)
+int membuf_free(struct membuf *mb)
 {
 	return mb->end == mb->start ? 0 :
 			(mb->end - mb->start) - 1 - membuf_avail(mb);
 }
 
-int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool must_fit)
+int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch, bool must_fit)
 {
 	int len;  /* number of bytes read (!= string length) */
 	char *s, *end;
@@ -322,7 +322,7 @@ int membuf_readline(struct membuff *mb, char *str, int maxlen, int minch, bool m
 	return len;
 }
 
-int membuf_extend_by(struct membuff *mb, int by, int max)
+int membuf_extend_by(struct membuf *mb, int by, int max)
 {
 	int oldhead, oldtail;
 	int size, orig;
@@ -358,14 +358,14 @@ int membuf_extend_by(struct membuff *mb, int by, int max)
 	return 0;
 }
 
-void membuf_init(struct membuff *mb, char *buff, int size)
+void membuf_init(struct membuf *mb, char *buff, int size)
 {
 	mb->start = buff;
 	mb->end = mb->start + size;
 	membuf_purge(mb);
 }
 
-int membuf_new(struct membuff *mb, int size)
+int membuf_new(struct membuf *mb, int size)
 {
 	mb->start = malloc(size);
 	if (!mb->start)
@@ -375,14 +375,14 @@ int membuf_new(struct membuff *mb, int size)
 	return 0;
 }
 
-void membuf_uninit(struct membuff *mb)
+void membuf_uninit(struct membuf *mb)
 {
 	mb->end = NULL;
 	mb->start = NULL;
 	membuf_purge(mb);
 }
 
-void membuf_dispose(struct membuff *mb)
+void membuf_dispose(struct membuf *mb)
 {
 	free(&mb->start);
 	membuf_uninit(mb);
-- 
2.34.1

Re: [PATCH 3/8] membuf: Rename struct

[Simon Glass]

Rename the struct to match the function prefix and filenames.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/include/asm/serial.h |  2 +-
 boot/bootmeth_extlinux.c          |  2 +-
 common/console.c                  | 24 +++++++++---------
 drivers/usb/emul/sandbox_keyb.c   |  2 +-
 include/asm-generic/global_data.h |  4 +--
 include/membuf.h                  | 42 +++++++++++++++----------------
 lib/membuf.c                      | 42 +++++++++++++++----------------
 7 files changed, 59 insertions(+), 59 deletions(-)

Applied to sjg/master, thanks!

[PATCH 4/8] membuf: Include stdbool

[Simon Glass]

This uses a bool type so include the required header.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/membuf.h b/include/membuf.h
index 17616d5577e..636ed703ee7 100644
--- a/include/membuf.h
+++ b/include/membuf.h
@@ -9,6 +9,8 @@
 #ifndef _membuf_H
 #define _membuf_H
 
+#include <stdbool.h>
+
 /**
  * @struct membuf: holds the state of a membuff - it is used for input and
  * output buffers. The buffer extends from @start to (@start + @size - 1).
-- 
2.34.1

Re: [PATCH 4/8] membuf: Include stdbool

[Simon Glass]

This uses a bool type so include the required header.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h | 2 ++
 1 file changed, 2 insertions(+)

Applied to sjg/master, thanks!

[PATCH 5/8] membuf: Correct implementation of membuf_dispose()

[Simon Glass]

This should free the pointer, not the address of the pointer. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/membuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/membuf.c b/lib/membuf.c
index b13998ccdbd..695d16d051e 100644
--- a/lib/membuf.c
+++ b/lib/membuf.c
@@ -384,6 +384,6 @@ void membuf_uninit(struct membuf *mb)
 
 void membuf_dispose(struct membuf *mb)
 {
-	free(&mb->start);
+	free(mb->start);
 	membuf_uninit(mb);
 }
-- 
2.34.1

Re: [PATCH 5/8] membuf: Correct implementation of membuf_dispose()

[Simon Glass]

This should free the pointer, not the address of the pointer. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 lib/membuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied to sjg/master, thanks!

[PATCH 6/8] membuf: Add some tests

[Simon Glass]

Add tests for the membuf implementation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 test/lib/Makefile |   1 +
 test/lib/membuf.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+)
 create mode 100644 test/lib/membuf.c

diff --git a/test/lib/Makefile b/test/lib/Makefile
index a54387a058e..014cd42efe1 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -12,6 +12,7 @@ obj-y += hexdump.o
 obj-$(CONFIG_SANDBOX) += kconfig.o
 obj-y += lmb.o
 obj-y += longjmp.o
+obj-$(CONFIG_SANDBOX) += membuf.o
 obj-$(CONFIG_CONSOLE_RECORD) += test_print.o
 obj-$(CONFIG_SSCANF) += sscanf.o
 obj-y += string.o
diff --git a/test/lib/membuf.c b/test/lib/membuf.c
new file mode 100644
index 00000000000..3f268a422d5
--- /dev/null
+++ b/test/lib/membuf.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <membuf.h>
+#include <os.h>
+#include <rand.h>
+#include <string.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+#define TEST_SIZE	16
+#define TEST_COUNT	10000
+
+static void membuf_zero(struct membuf *mb)
+{
+	memset(mb->start, '\0', mb->end - mb->start);
+}
+
+static int membuf_check(struct unit_test_state *uts, struct membuf *mb,
+			int value)
+{
+	/* head is out of range */
+	ut_assert(!(mb->head < mb->start || mb->head >= mb->end));
+
+	/* tail is out of range */
+	ut_assert(!(mb->tail < mb->start || mb->tail >= mb->end));
+
+	return 0;
+}
+
+/* write from 1 to test_size bytes, and check they come back OK */
+static int lib_test_membuf_one(struct unit_test_state *uts)
+{
+	char in[TEST_SIZE * 2], out[TEST_SIZE * 2];
+	struct membuf mb;
+	int size, ret, test_size, i;
+
+	ut_assertok(membuf_new(&mb, TEST_SIZE));
+
+	/* setup in test */
+	for (i = 0; i < TEST_SIZE; i++) {
+		in[i] = (i & 63) + '0';
+		in[i + TEST_SIZE] = in[i];
+	}
+
+	test_size = TEST_SIZE;
+
+	for (i = 1; i < TEST_COUNT; i++) {
+		membuf_zero(&mb);
+		size = rand() % test_size;
+
+		// now write patterns and check they come back OK
+		ret = membuf_put(&mb, in, 0);
+		ret = membuf_put(&mb, in, size);
+		ut_asserteq(size, ret);
+
+		ret = membuf_put(&mb, in, 0);
+		ut_assertok(membuf_check(uts, &mb, i));
+
+		ret = membuf_get(&mb, out, 0);
+		ret = membuf_get(&mb, out, size);
+		ut_asserteq(size, ret);
+
+		ret = membuf_get(&mb, out, 0);
+		ut_assertok(membuf_check(uts, &mb, i));
+
+		ut_asserteq_mem(in, out, size);
+	}
+
+	return 0;
+}
+LIB_TEST(lib_test_membuf_one, 0);
+
+/* write random number of bytes, and check they come back OK */
+static int lib_test_membuf_random(struct unit_test_state *uts)
+{
+	char in[TEST_SIZE * 2];
+	char buf[TEST_SIZE * 2];
+	struct membuf mb;
+	int size, ret, test_size, i;
+	char *inptr, *outptr;
+	int max_avail, min_free;
+
+	ut_assertok(membuf_new(&mb, TEST_SIZE));
+
+	for (i = 0; i < TEST_SIZE; i++) {
+		in[i] = (i & 63) + '0';
+		in[i + TEST_SIZE] = in[i];
+	}
+
+	test_size = TEST_SIZE;
+
+	inptr = in;
+	outptr = in;
+	min_free = TEST_COUNT;
+	max_avail = 0;
+	membuf_zero(&mb);
+	for (i = 0; i < TEST_COUNT; i++) {
+		size = rand() % test_size;
+
+		if (membuf_free(&mb) < min_free)
+			min_free = membuf_free(&mb);
+
+		ret = membuf_put(&mb, inptr, size);
+		ut_assertok(membuf_check(uts, &mb, i));
+		inptr += ret;
+		if (inptr >= in + TEST_SIZE)
+			inptr -= TEST_SIZE;
+
+		size = rand() % (test_size - 1);
+
+		if (membuf_avail(&mb) > max_avail)
+			max_avail = membuf_avail(&mb);
+
+		ret = membuf_get(&mb, buf, size);
+		ut_assertok(membuf_check(uts, &mb, i));
+		ut_asserteq_mem(buf, outptr, ret);
+
+		outptr += ret;
+		if (outptr >= in + TEST_SIZE)
+			outptr -= TEST_SIZE;
+	}
+
+	return 0;
+}
+LIB_TEST(lib_test_membuf_random, 0);
+
+/* test membuf_extend() with split segments */
+static int lib_test_membuf_extend(struct unit_test_state *uts)
+{
+	char in[TEST_SIZE * 2];
+	char buf[TEST_SIZE * 2];
+	struct membuf mb;
+	int ret, test_size, i, cur;
+	char *data;
+
+	ut_assertok(membuf_new(&mb, TEST_SIZE));
+
+	for (i = 0; i < TEST_SIZE; i++) {
+		in[i] = (i & 63) + '0';
+		in[i + TEST_SIZE] = in[i];
+	}
+
+	test_size = TEST_SIZE - 1;
+
+	for (cur = 0; cur <= test_size; cur++) {
+		ut_assertok(membuf_new(&mb, TEST_SIZE));
+
+		membuf_zero(&mb);
+
+		/*
+		 * add some bytes, then remove them - this will force the membuf
+		 * to have data split into two segments when we fill it
+		 */
+		ret = membuf_putraw(&mb, TEST_SIZE / 2, true, &data);
+		membuf_getraw(&mb, ret, true, &data);
+		ut_asserteq(TEST_SIZE / 2, ret);
+
+		/* fill it */
+		ret = membuf_put(&mb, in, cur);
+		ut_assertok(membuf_check(uts, &mb, cur));
+		ut_asserteq(cur, ret);
+
+		/* extend the buffer */
+		ut_assertok(membuf_extend_by(&mb, TEST_SIZE, -1));
+		ut_assertok(membuf_check(uts, &mb, cur));
+
+		/* check our data is still there */
+		ret = membuf_get(&mb, buf, TEST_SIZE * 2);
+		ut_assertok(membuf_check(uts, &mb, cur));
+		ut_asserteq(cur, ret);
+		ut_asserteq_mem(in, buf, cur);
+		membuf_uninit(&mb);
+	}
+
+	return 0;
+}
+LIB_TEST(lib_test_membuf_extend, 0);
+
+/* test membuf_readline() with generated data */
+static int lib_test_membuf_readline(struct unit_test_state *uts)
+{
+	char *buf;
+	int size, cur, i, ret, readptr, cmpptr;
+	struct membuf mb;
+	char *data;
+	char str[256];
+	char *s;
+
+	ut_assertok(membuf_new(&mb, 1024));
+	membuf_zero(&mb);
+
+	/* Use the README as test data */
+	ut_assertok(os_read_file("README", (void **)&buf, &size));
+
+	cur = 0;
+	readptr = 0;
+	cmpptr = 0;
+	for (i = 0; i < 100000; i++, cur += 1) {
+		/* fill the buffer with up to 'cur' bytes */
+		ret = membuf_putraw(&mb, cur, false, &data);
+
+		if (ret > 0) {
+			int can_read = min(ret, size - readptr);
+
+			memcpy(data, &buf[readptr], can_read);
+			readptr += can_read;
+
+			membuf_putraw(&mb, can_read, true, &data);
+			ut_assertok(membuf_check(uts, &mb, i));
+		}
+
+		/* read a line and compare */
+		ret = membuf_readline(&mb, str, 256, 0, true);
+		ut_assertok(membuf_check(uts, &mb, i));
+		if (ret) {
+			char *ptr;
+
+			s = &buf[cmpptr];
+			ptr = strchr(s, '\n');
+			*ptr = '\0';
+
+			ut_asserteq_str(s, str);
+			cmpptr += strlen(s) + 1;
+			*ptr = '\n';
+		} else {
+			ut_assert(membuf_free(&mb));
+		}
+	}
+	membuf_dispose(&mb);
+	os_free(buf);
+
+	return 0;
+}
+LIB_TEST(lib_test_membuf_readline, 0);
-- 
2.34.1

Re: [PATCH 6/8] membuf: Add some tests

[Simon Glass]

Add tests for the membuf implementation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 test/lib/Makefile |   1 +
 test/lib/membuf.c | 239 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 240 insertions(+)
 create mode 100644 test/lib/membuf.c

Applied to sjg/master, thanks!

[PATCH 7/8] membuf: Minor code-style improvements

[Simon Glass]

Show the start in end in the comment. Comment a missing variable in
membuf_readline() and fix its line length.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h | 6 ++++--
 lib/membuf.c     | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/membuf.h b/include/membuf.h
index 636ed703ee7..46764690f53 100644
--- a/include/membuf.h
+++ b/include/membuf.h
@@ -25,7 +25,7 @@
  *
  * .............xxxxxxxxxxxxxxxx.........................
  *		^		^
- *		tail		head
+ * ^start	tail		head                     ^end
  *
  * xxxxxxxxxxxxx................xxxxxxxxxxxxxxxxxxxxxxxxx
  *		^		^
@@ -194,11 +194,13 @@ int membuf_free(struct membuf *mb);
  * @mb: membuff to adjust
  * @str: Place to put the line
  * @maxlen: Maximum line length (excluding terminator)
+ * @minch: Minimum ASCII character to permit as part of the line (e.g. ' ')
  * @must_fit: If true then str is empty if line doesn't fit
  * Return: number of bytes read (including terminator) if a line has been
  *	   read, 0 if nothing was there or line didn't fit when must_fit is set
  */
-int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch, bool must_fit);
+int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
+		    bool must_fit);
 
 /**
  * membuf_extend_by() - expand a membuff
diff --git a/lib/membuf.c b/lib/membuf.c
index 695d16d051e..f38ff36cb0b 100644
--- a/lib/membuf.c
+++ b/lib/membuf.c
@@ -287,7 +287,8 @@ int membuf_free(struct membuf *mb)
 			(mb->end - mb->start) - 1 - membuf_avail(mb);
 }
 
-int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch, bool must_fit)
+int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
+		    bool must_fit)
 {
 	int len;  /* number of bytes read (!= string length) */
 	char *s, *end;
-- 
2.34.1

Re: [PATCH 7/8] membuf: Minor code-style improvements

[Simon Glass]

Show the start in end in the comment. Comment a missing variable in
membuf_readline() and fix its line length.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h | 6 ++++--
 lib/membuf.c     | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

Applied to sjg/master, thanks!

[PATCH 8/8] membuf: Support a flag for being full

[Simon Glass]

At present the membuf implementation wastes a slot in the fifo so it can
detect the difference between a full and an empty buffer.

Add the option of supporting a boolean flag, if desired. For now it is
off.

The code-size penalty is non-zero, but the space penalty is small and
could be reduced on 64-bit machines by using a u32 offset for head and
tail.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h  |  6 +++++
 lib/membuf.c      | 63 ++++++++++++++++++++++++++++++++++++++---------
 test/lib/membuf.c | 11 ++++++---
 3 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/include/membuf.h b/include/membuf.h
index 46764690f53..b495a72652b 100644
--- a/include/membuf.h
+++ b/include/membuf.h
@@ -11,6 +11,9 @@
 
 #include <stdbool.h>
 
+/* Set this to 1 to support a 'full' flag */
+#define MEMBUF_FULL	0
+
 /**
  * @struct membuf: holds the state of a membuff - it is used for input and
  * output buffers. The buffer extends from @start to (@start + @size - 1).
@@ -36,6 +39,9 @@ struct membuf {
 	char *end;		/** the end of the buffer (start + length) */
 	char *head;		/** current buffer head */
 	char *tail;		/** current buffer tail */
+#if MEMBUF_FULL
+	bool full;		/** true if full (for when head == tail) */
+#endif
 };
 
 /**
diff --git a/lib/membuf.c b/lib/membuf.c
index f38ff36cb0b..016430ae988 100644
--- a/lib/membuf.c
+++ b/lib/membuf.c
@@ -11,11 +11,28 @@
 #include <malloc.h>
 #include "membuf.h"
 
+static inline bool is_full(const struct membuf *mb)
+{
+#if MEMBUF_FULL
+	return mb->full;
+#else
+	return false;
+#endif
+}
+
+static inline void set_full(struct membuf *mb, bool full)
+{
+#if MEMBUF_FULL
+	mb->full = full;
+#endif
+}
+
 void membuf_purge(struct membuf *mb)
 {
 	/* set mb->head and mb->tail so the buffers look empty */
 	mb->head = mb->start;
 	mb->tail = mb->start;
+	set_full(mb, false);
 }
 
 static int membuf_putrawflex(struct membuf *mb, int maxlen, bool update,
@@ -36,21 +53,29 @@ static int membuf_putrawflex(struct membuf *mb, int maxlen, bool update,
 	 * if head is ahead of tail, we can write from head until the end of
 	 * the buffer
 	 */
-	if (mb->head >= mb->tail) {
+	if (mb->head >= mb->tail && !is_full(mb)) {
 		/* work out how many bytes can fit here */
-		len = mb->end - mb->head - 1;
+		len = mb->end - mb->head;
+		if (!MEMBUF_FULL)
+			len--;
 		if (maxlen >= 0 && len > maxlen)
 			len = maxlen;
 
 		/* update the head pointer to mark these bytes as written */
-		if (update)
+		if (update) {
 			mb->head += len;
+			if (mb->head == mb->end)
+				mb->head = mb->start;
+			if (len && mb->head == mb->tail)
+				set_full(mb, true);
+		}
 
 		/*
 		 * if the tail isn't at start of the buffer, then we can
 		 * write one more byte right at the end
 		 */
-		if ((maxlen < 0 || len < maxlen) && mb->tail != mb->start) {
+		if (!MEMBUF_FULL && (maxlen < 0 || len < maxlen) &&
+		    mb->tail != mb->start) {
 			len++;
 			if (update)
 				mb->head = mb->start;
@@ -59,13 +84,18 @@ static int membuf_putrawflex(struct membuf *mb, int maxlen, bool update,
 	/* otherwise now we can write until head almost reaches tail */
 	} else {
 		/* work out how many bytes can fit here */
-		len = mb->tail - mb->head - 1;
+		len = mb->tail - mb->head;
+		if (!MEMBUF_FULL)
+			len--;
 		if (maxlen >= 0 && len > maxlen)
 			len = maxlen;
 
 		/* update the head pointer to mark these bytes as written */
-		if (update)
+		if (update) {
 			mb->head += len;
+			if (MEMBUF_FULL && len && mb->head == mb->tail)
+				set_full(mb, true);
+		}
 	}
 
 	/* return the number of bytes which can be/must be written */
@@ -125,7 +155,7 @@ int membuf_getraw(struct membuf *mb, int maxlen, bool update, char **data)
 	 * and some more data between 'start' and 'head'(which we can't
 	 * return this time
 	 */
-	else if (mb->head < mb->tail) {
+	else if (is_full(mb) || mb->head < mb->tail) {
 		/* work out the amount of data */
 		*data = mb->tail;
 		len = mb->end - mb->tail;
@@ -135,6 +165,8 @@ int membuf_getraw(struct membuf *mb, int maxlen, bool update, char **data)
 			mb->tail += len;
 			if (mb->tail == mb->end)
 				mb->tail = mb->start;
+			if (len)
+				set_full(mb, false);
 		}
 	}
 
@@ -205,7 +237,7 @@ int membuf_put(struct membuf *mb, const char *buff, int length)
 
 bool membuf_isempty(struct membuf *mb)
 {
-	return mb->head == mb->tail;
+	return !is_full(mb) && mb->head == mb->tail;
 }
 
 int membuf_avail(struct membuf *mb)
@@ -230,6 +262,9 @@ int membuf_size(struct membuf *mb)
 	return mb->end - mb->start;
 }
 
+#if MEMBUF_FULL == 0
+/* This has not been updated for MEMBUF_FULL */
+
 bool membuf_makecontig(struct membuf *mb)
 {
 	int topsize, botsize;
@@ -280,11 +315,12 @@ bool membuf_makecontig(struct membuf *mb)
 	/* all ok */
 	return true;
 }
+#endif
 
 int membuf_free(struct membuf *mb)
 {
 	return mb->end == mb->start ? 0 :
-			(mb->end - mb->start) - 1 - membuf_avail(mb);
+		(mb->end - mb->start) - MEMBUF_FULL - membuf_avail(mb);
 }
 
 int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
@@ -295,7 +331,7 @@ int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
 	bool ok = false;
 	char *orig = str;
 
-	end = mb->head >= mb->tail ? mb->head : mb->end;
+	end = mb->head < mb->tail || is_full(mb) ? mb->end : mb->head;
 	for (len = 0, s = mb->tail; s < end && len < maxlen - 1; str++) {
 		*str = *s++;
 		len++;
@@ -303,7 +339,7 @@ int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
 			ok = true;
 			break;
 		}
-		if (s == end && mb->tail > mb->head) {
+		if (s == end && (is_full(mb) || mb->tail > mb->head)) {
 			s = mb->start;
 			end = mb->head;
 		}
@@ -319,6 +355,8 @@ int membuf_readline(struct membuf *mb, char *str, int maxlen, int minch,
 	/* terminate the string, update the membuff and return success */
 	*str = '\0';
 	mb->tail = s == mb->end ? mb->start : s;
+	if (len)
+		set_full(mb, false);
 
 	return len;
 }
@@ -350,9 +388,10 @@ int membuf_extend_by(struct membuf *mb, int by, int max)
 	mb->head = mb->start + oldhead;
 	mb->tail = mb->start + oldtail;
 
-	if (mb->head < mb->tail) {
+	if (is_full(mb) || mb->head < mb->tail) {
 		memmove(mb->tail + by, mb->tail, orig - oldtail);
 		mb->tail += by;
+		set_full(mb, false);
 	}
 	mb->end = mb->start + size;
 
diff --git a/test/lib/membuf.c b/test/lib/membuf.c
index 3f268a422d5..f36332ff7b6 100644
--- a/test/lib/membuf.c
+++ b/test/lib/membuf.c
@@ -23,6 +23,11 @@ static void membuf_zero(struct membuf *mb)
 static int membuf_check(struct unit_test_state *uts, struct membuf *mb,
 			int value)
 {
+	/* says it is full, but can't be */
+#if MEMBUF_FULL
+	ut_assert(!(mb->full && mb->head != mb->tail));
+#endif
+
 	/* head is out of range */
 	ut_assert(!(mb->head < mb->start || mb->head >= mb->end));
 
@@ -47,7 +52,7 @@ static int lib_test_membuf_one(struct unit_test_state *uts)
 		in[i + TEST_SIZE] = in[i];
 	}
 
-	test_size = TEST_SIZE;
+	test_size = TEST_SIZE + MEMBUF_FULL;
 
 	for (i = 1; i < TEST_COUNT; i++) {
 		membuf_zero(&mb);
@@ -92,7 +97,7 @@ static int lib_test_membuf_random(struct unit_test_state *uts)
 		in[i + TEST_SIZE] = in[i];
 	}
 
-	test_size = TEST_SIZE;
+	test_size = TEST_SIZE + MEMBUF_FULL;
 
 	inptr = in;
 	outptr = in;
@@ -145,7 +150,7 @@ static int lib_test_membuf_extend(struct unit_test_state *uts)
 		in[i + TEST_SIZE] = in[i];
 	}
 
-	test_size = TEST_SIZE - 1;
+	test_size = TEST_SIZE - 1 + MEMBUF_FULL;
 
 	for (cur = 0; cur <= test_size; cur++) {
 		ut_assertok(membuf_new(&mb, TEST_SIZE));
-- 
2.34.1

Re: [PATCH 8/8] membuf: Support a flag for being full

[Simon Glass]

At present the membuf implementation wastes a slot in the fifo so it can
detect the difference between a full and an empty buffer.

Add the option of supporting a boolean flag, if desired. For now it is
off.

The code-size penalty is non-zero, but the space penalty is small and
could be reduced on 64-bit machines by using a u32 offset for head and
tail.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/membuf.h  |  6 +++++
 lib/membuf.c      | 63 ++++++++++++++++++++++++++++++++++++++---------
 test/lib/membuf.c | 11 ++++++---
 3 files changed, 65 insertions(+), 15 deletions(-)

Applied to sjg/master, thanks!

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Rasmus Villemoes]

On tor, okt 17 2024, Simon Glass <sjg@chromium.org> wrote:

>
>   membuf: Support a flag for being full
>

No, that is the worst of all worlds, especially with it being a
build-time flag. The right implementation is the one where the head and
tail indices are free-running, where you get such a "flag" for
free, because you're not wasting the top bits of the indices.

https://www.snellman.net/blog/archive/2016-12-13-ring-buffers/

If you want to do the churn of renaming anyway, I suggest doing it by
adding an implementation using the proper scheme under the new name,
switch users over, and dropping the old. IMO, this series as-is brings
no value (except for the tests, of course).

Rasmus

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Simon Glass]

Hi Rasmus,

On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On tor, okt 17 2024, Simon Glass <sjg@chromium.org> wrote:
>
> >
> >   membuf: Support a flag for being full
> >
>
> No, that is the worst of all worlds, especially with it being a
> build-time flag. The right implementation is the one where the head and
> tail indices are free-running, where you get such a "flag" for
> free, because you're not wasting the top bits of the indices.
>
> https://www.snellman.net/blog/archive/2016-12-13-ring-buffers/

Thanks for the link. It is an interesting read, including the
comments. I've only written one ring buffer in my life and this is it
:-)

Which option do you suggest? We don't currently restrict the size to a
power of two and that does seem like a strange restriction. It will
make it less useful, e.g. in extlinux_fill_info() and potentially
replacing memgets().

We actually don't have to worry (today) about threading, so option two
could work, I suppose. I don't like the 'length' field any more than
the author, but we could have a simple function to turn it into
(pointer plus an empty/full flag)

The power-of-two restriction could be lifted by use division instead
of masking and making sure that the indexes keep to reasonable values
(say no more than twice the buffer size). In fact with the limit, we
could just subtract the length from the index when accessing it, if
needed.

But overall I think I'm with Chris.

>
> If you want to do the churn of renaming anyway, I suggest doing it by
> adding an implementation using the proper scheme under the new name,
> switch users over, and dropping the old. IMO, this series as-is brings
> no value (except for the tests, of course).

OK. Do you think this series gets us closer to that, or further away?

Regards,
Simon

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Simon Glass]

Hi Rasmus,

On Fri, 18 Oct 2024 at 08:55, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Rasmus,
>
> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >
> > On tor, okt 17 2024, Simon Glass <sjg@chromium.org> wrote:
> >
> > >
> > >   membuf: Support a flag for being full
> > >
> >
> > No, that is the worst of all worlds, especially with it being a
> > build-time flag. The right implementation is the one where the head and
> > tail indices are free-running, where you get such a "flag" for
> > free, because you're not wasting the top bits of the indices.
> >
> > https://www.snellman.net/blog/archive/2016-12-13-ring-buffers/
>
> Thanks for the link. It is an interesting read, including the
> comments. I've only written one ring buffer in my life and this is it
> :-)
>
> Which option do you suggest? We don't currently restrict the size to a
> power of two and that does seem like a strange restriction. It will
> make it less useful, e.g. in extlinux_fill_info() and potentially
> replacing memgets().
>
> We actually don't have to worry (today) about threading, so option two
> could work, I suppose. I don't like the 'length' field any more than
> the author, but we could have a simple function to turn it into
> (pointer plus an empty/full flag)
>
> The power-of-two restriction could be lifted by use division instead
> of masking and making sure that the indexes keep to reasonable values
> (say no more than twice the buffer size). In fact with the limit, we
> could just subtract the length from the index when accessing it, if
> needed.
>
> But overall I think I'm with Chris.
>
> >
> > If you want to do the churn of renaming anyway, I suggest doing it by
> > adding an implementation using the proper scheme under the new name,
> > switch users over, and dropping the old. IMO, this series as-is brings
> > no value (except for the tests, of course).
>
> OK. Do you think this series gets us closer to that, or further away?

I didn't get a response to this (which is not a problem, I miss things
all the time). Anyway I don't like the power-of-two restriction and
you can see my other responses above. I've applied this to my tree as
I want to have the tests in place.

Regards,
SImon

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Rasmus Villemoes]

On Tue, Mar 04 2025, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On Fri, 18 Oct 2024 at 08:55, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Rasmus,
>>
>> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> >
>> > On tor, okt 17 2024, Simon Glass <sjg@chromium.org> wrote:
>> >
>> > >
>> > >   membuf: Support a flag for being full
>> > >
>> >
>> > No, that is the worst of all worlds, especially with it being a
>> > build-time flag. The right implementation is the one where the head and
>> > tail indices are free-running, where you get such a "flag" for
>> > free, because you're not wasting the top bits of the indices.
>> >
>> > https://www.snellman.net/blog/archive/2016-12-13-ring-buffers/
>>
>> >
>> > If you want to do the churn of renaming anyway, I suggest doing it by
>> > adding an implementation using the proper scheme under the new name,
>> > switch users over, and dropping the old. IMO, this series as-is brings
>> > no value (except for the tests, of course).
>>
>> OK. Do you think this series gets us closer to that, or further away?
>
> I didn't get a response to this (which is not a problem, I miss things
> all the time). Anyway I don't like the power-of-two restriction and
> you can see my other responses above. I've applied this to my tree as
> I want to have the tests in place.

I stand by my earlier comments that this is the wrong way to implement a
circular buffer. I hope Tom doesn't pull this.

Rasmus

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Simon Glass]

Hi Rasmus,

On Tue, 4 Mar 2025 at 11:55, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Tue, Mar 04 2025, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On Fri, 18 Oct 2024 at 08:55, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Rasmus,
> >>
> >> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >> >
> >> > On tor, okt 17 2024, Simon Glass <sjg@chromium.org> wrote:
> >> >
> >> > >
> >> > >   membuf: Support a flag for being full
> >> > >
> >> >
> >> > No, that is the worst of all worlds, especially with it being a
> >> > build-time flag. The right implementation is the one where the head and
> >> > tail indices are free-running, where you get such a "flag" for
> >> > free, because you're not wasting the top bits of the indices.
> >> >
> >> > https://www.snellman.net/blog/archive/2016-12-13-ring-buffers/
> >>
> >> >
> >> > If you want to do the churn of renaming anyway, I suggest doing it by
> >> > adding an implementation using the proper scheme under the new name,
> >> > switch users over, and dropping the old. IMO, this series as-is brings
> >> > no value (except for the tests, of course).
> >>
> >> OK. Do you think this series gets us closer to that, or further away?
> >
> > I didn't get a response to this (which is not a problem, I miss things
> > all the time). Anyway I don't like the power-of-two restriction and
> > you can see my other responses above. I've applied this to my tree as
> > I want to have the tests in place.
>
> I stand by my earlier comments that this is the wrong way to implement a
> circular buffer. I hope Tom doesn't pull this.

OK. Are you saying that you think it should only support power-of-two
sizes, or something else? What specifically do you want?

This series:
- adds tests
- renames to membuf
- shows how we could switch to using an empty/full flag instead of
leaving an empty slot, so we can see the code-size image
- does all this without requiring the size to be a power of two (which
limits its usefulness IMO)

Regards,
Simon

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Rasmus Villemoes]

On Wed, Mar 05 2025, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On Tue, 4 Mar 2025 at 11:55, Rasmus Villemoes <ravi@prevas.dk> wrote:
>>
>> On Tue, Mar 04 2025, Simon Glass <sjg@chromium.org> wrote:
>>
>> > Hi Rasmus,
>> >
>> > On Fri, 18 Oct 2024 at 08:55, Simon Glass <sjg@chromium.org> wrote:
>> >>
>> >> Hi Rasmus,
>> >>
>> >> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
>> >> >
>> >> >
>> >> > If you want to do the churn of renaming anyway, I suggest doing it by
>> >> > adding an implementation using the proper scheme under the new name,
>> >> > switch users over, and dropping the old. IMO, this series as-is brings
>> >> > no value (except for the tests, of course).
>> >>
>> >> OK. Do you think this series gets us closer to that, or further away?
>> >
>> > I didn't get a response to this (which is not a problem, I miss things
>> > all the time). Anyway I don't like the power-of-two restriction and
>> > you can see my other responses above. I've applied this to my tree as
>> > I want to have the tests in place.
>>
>> I stand by my earlier comments that this is the wrong way to implement a
>> circular buffer. I hope Tom doesn't pull this.
>
> OK. Are you saying that you think it should only support power-of-two
> sizes,

Yes, because that's the natural way to implement such a simple data
structure on real hardware, and I don't agree that it limits its
usefulness in any way. See how the linux kernel implements unix pipes,
and their kfifo helper.

> or something else? What specifically do you want?

I don't "want" anything in particular. I'm merely voicing my opinion
that I consider this approach to implementing a circular buffer
inferior.

> This series:
> - adds tests

Yes, always a good thing.

> - renames to membuf

While we can agree that is better than with two f's, I don't see it as a
huge virtue.

> - shows how we could switch to using an empty/full flag instead of
> leaving an empty slot, so we can see the code-size image

As I've said previously, I believe that actually makes the whole thing
even worse. Not using the natural wrapping/masking is already
error-prone enough, but needing to have two parallel implementations
living in the same source files with #ifdeffery; please, no. Do one or
the other, unconditionally.

> - does all this without requiring the size to be a power of two (which
> limits its usefulness IMO)

On that, we'll just have to agree to disagree.

Rasmus

PS: Something like
https://gist.github.com/Villemoes/332ac7a5dfc983c58ad40773c7bc6385 is
what I consider a simple and readable implementation (it may be buggy,
of course).

Frankly, the triple-pointer use in membuff.c frightens me, and that the
implementation of something as simple as membuff_avail() involves a
struct copy and calling membuff_getraw() in a loop makes it really hard
to reason about and verify.

Re: [PATCH 0/8] membuff: Add tests and update to support a flag for empty/full

[Simon Glass]

Hi Rasmus,

On Thu, 6 Mar 2025 at 07:21, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Wed, Mar 05 2025, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On Tue, 4 Mar 2025 at 11:55, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >>
> >> On Tue, Mar 04 2025, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> > Hi Rasmus,
> >> >
> >> > On Fri, 18 Oct 2024 at 08:55, Simon Glass <sjg@chromium.org> wrote:
> >> >>
> >> >> Hi Rasmus,
> >> >>
> >> >> On Fri, 18 Oct 2024 at 01:05, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >> >> >
> >> >> >
> >> >> > If you want to do the churn of renaming anyway, I suggest doing it by
> >> >> > adding an implementation using the proper scheme under the new name,
> >> >> > switch users over, and dropping the old. IMO, this series as-is brings
> >> >> > no value (except for the tests, of course).
> >> >>
> >> >> OK. Do you think this series gets us closer to that, or further away?
> >> >
> >> > I didn't get a response to this (which is not a problem, I miss things
> >> > all the time). Anyway I don't like the power-of-two restriction and
> >> > you can see my other responses above. I've applied this to my tree as
> >> > I want to have the tests in place.
> >>
> >> I stand by my earlier comments that this is the wrong way to implement a
> >> circular buffer. I hope Tom doesn't pull this.
> >
> > OK. Are you saying that you think it should only support power-of-two
> > sizes,
>
> Yes, because that's the natural way to implement such a simple data
> structure on real hardware, and I don't agree that it limits its
> usefulness in any way. See how the linux kernel implements unix pipes,
> and their kfifo helper.
>
> > or something else? What specifically do you want?
>
> I don't "want" anything in particular. I'm merely voicing my opinion
> that I consider this approach to implementing a circular buffer
> inferior.
>
> > This series:
> > - adds tests
>
> Yes, always a good thing.
>
> > - renames to membuf
>
> While we can agree that is better than with two f's, I don't see it as a
> huge virtue.
>
> > - shows how we could switch to using an empty/full flag instead of
> > leaving an empty slot, so we can see the code-size image
>
> As I've said previously, I believe that actually makes the whole thing
> even worse. Not using the natural wrapping/masking is already
> error-prone enough, but needing to have two parallel implementations
> living in the same source files with #ifdeffery; please, no. Do one or
> the other, unconditionally.

Sure, I can remove that, since it seems that the flag is not that
useful. I'll send a patch.

>
> > - does all this without requiring the size to be a power of two (which
> > limits its usefulness IMO)
>
> On that, we'll just have to agree to disagree.

An example of this is parsing a text file. At present you can just:

membuf_init(size...)
membuf_readline()...

whereas presumably with power-of-two you would need to

int newsize = ALIGN(size, next power of two....)

void *newptr = malloc(newsize)
memcpy(newptr, data, size)
membuf_init(newsize...)
membuf_setup([so that only 'size' bytes are present])
membuf_readline()...

>
> Rasmus
>
> PS: Something like
> https://gist.github.com/Villemoes/332ac7a5dfc983c58ad40773c7bc6385 is
> what I consider a simple and readable implementation (it may be buggy,
> of course).

There are no tests, so who knows? But also, it doesn't allow 'raw'
access which is considerably more efficient in many situations, e.g.
you can find out the amount of space, read directly into that space
without a separate buffer, then confirm how much you read.

Certainly my code can be improved.

>
> Frankly, the triple-pointer use in membuff.c frightens me, and that the
> implementation of something as simple as membuff_avail() involves a
> struct copy and calling membuff_getraw() in a loop makes it really hard
> to reason about and verify.

We have tests for verification. But how about you send a patch against
the API in my tree? If we can come up with something we both like,
then perhaps Tom might apply it.

Regards,
Simon