[PATCH 0/9] fifo8: add fifo8_peek(), fifo8_peek_buf() and tests

[PATCH 0/9] fifo8: add fifo8_peek(), fifo8_peek_buf() and tests

[Mark Cave-Ayland]

This is something I've had lying around for a little while as a follow on from
Phil's recent work on Fifo8 with a few updates, but also adding the missing
fifo8_peek() and fifo8_peek_buf() functions along with some relevant tests.

The reason for sending this now is that there are couple of recent series
(https://patchew.org/QEMU/20240819113148.3007047-1-alistair.francis@wdc.com/ and
https://patchew.org/QEMU/20240817102606.3996242-1-tavip@google.com/) which can
benefit from these changes: in particular the fifo8_peek_buf() function, unlike the
existing fifo8_peek_bufptr() function, will correctly handle FIFO wraparound. This
occurs when the FIFO head drifts due to not popping the entire FIFO content in one
go, which often happens when trying to send FIFO data to a chardev.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (9):
  fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()
  fifo8: introduce head variable for fifo8_peekpop_bufptr()
  fifo8: add skip parameter to fifo8_peekpop_bufptr()
  fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in
    fifo8_pop_buf()
  fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()
  fifo8: honour do_pop argument in fifo8_peekpop_buf()
  fifo8: add fifo8_peek_buf() function
  fifo8: introduce fifo8_peek() function
  tests/unit: add test-fifo unit test

 include/qemu/fifo8.h   |  25 ++++
 tests/unit/meson.build |   1 +
 tests/unit/test-fifo.c | 256 +++++++++++++++++++++++++++++++++++++++++
 util/fifo8.c           |  42 +++++--
 4 files changed, 313 insertions(+), 11 deletions(-)
 create mode 100644 tests/unit/test-fifo.c

-- 
2.39.2

[PATCH 1/9] fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()

[Mark Cave-Ayland]

This is to emphasise that the function returns a pointer to the internal FIFO
buffer.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index 1ffa19d900..61bce9d9a0 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -71,8 +71,8 @@ uint8_t fifo8_pop(Fifo8 *fifo)
     return ret;
 }
 
-static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
-                                        uint32_t *numptr, bool do_pop)
+static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
+                                           uint32_t *numptr, bool do_pop)
 {
     uint8_t *ret;
     uint32_t num;
@@ -94,12 +94,12 @@ static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
 
 const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
 {
-    return fifo8_peekpop_buf(fifo, max, numptr, false);
+    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
 }
 
 const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
 {
-    return fifo8_peekpop_buf(fifo, max, numptr, true);
+    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
 }
 
 uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
-- 
2.39.2

[PATCH 2/9] fifo8: introduce head variable for fifo8_peekpop_bufptr()

[Mark Cave-Ayland]

Rather than operate on fifo->head directly, introduce a new head variable which is
set to the value of fifo->head and use it instead. This is to allow future
adjustment of the head position within the internal FIFO buffer.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index 61bce9d9a0..5faa814a6e 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -75,11 +75,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
                                            uint32_t *numptr, bool do_pop)
 {
     uint8_t *ret;
-    uint32_t num;
+    uint32_t num, head;
 
     assert(max > 0 && max <= fifo->num);
-    num = MIN(fifo->capacity - fifo->head, max);
-    ret = &fifo->data[fifo->head];
+    head = fifo->head;
+    num = MIN(fifo->capacity - head, max);
+    ret = &fifo->data[head];
 
     if (do_pop) {
         fifo->head += num;
-- 
2.39.2

[PATCH 3/9] fifo8: add skip parameter to fifo8_peekpop_bufptr()

[Mark Cave-Ayland]

The skip parameter specifies the number of bytes to be skipped from the current
FIFO head before the peek or pop operation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index 5faa814a6e..62d6430b05 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -72,18 +72,20 @@ uint8_t fifo8_pop(Fifo8 *fifo)
 }
 
 static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
-                                           uint32_t *numptr, bool do_pop)
+                                           uint32_t skip, uint32_t *numptr,
+                                           bool do_pop)
 {
     uint8_t *ret;
     uint32_t num, head;
 
     assert(max > 0 && max <= fifo->num);
-    head = fifo->head;
+    assert(skip <= fifo->num);
+    head = (fifo->head + skip) % fifo->capacity;
     num = MIN(fifo->capacity - head, max);
     ret = &fifo->data[head];
 
     if (do_pop) {
-        fifo->head += num;
+        fifo->head = head + num;
         fifo->head %= fifo->capacity;
         fifo->num -= num;
     }
@@ -95,12 +97,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
 
 const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
 {
-    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
+    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, false);
 }
 
 const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
 {
-    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
+    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
 }
 
 uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
-- 
2.39.2

[PATCH 4/9] fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in fifo8_pop_buf()

[Mark Cave-Ayland]

The upcoming peek functionality will require passing a non-zero value to
fifo8_peekpop_bufptr().

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index 62d6430b05..efe0117b1f 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -116,7 +116,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
     }
 
     len = destlen;
-    buf = fifo8_pop_bufptr(fifo, len, &n1);
+    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
     if (dest) {
         memcpy(dest, buf, n1);
     }
@@ -125,7 +125,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
     len -= n1;
     len = MIN(len, fifo8_num_used(fifo));
     if (len) {
-        buf = fifo8_pop_bufptr(fifo, len, &n2);
+        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
         if (dest) {
             memcpy(&dest[n1], buf, n2);
         }
-- 
2.39.2

[PATCH 5/9] fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()

[Mark Cave-Ayland]

The fifo8_pop_buf() function will soon also be used for peek operations, so rename
the function accordingly. Create a new fifo8_pop_buf() wrapper function that can
be used by existing callers.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index efe0117b1f..5453cbc1b0 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -105,7 +105,8 @@ const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
     return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
 }
 
-uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
+static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
+                                  bool do_pop)
 {
     const uint8_t *buf;
     uint32_t n1, n2 = 0;
@@ -134,6 +135,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
     return n1 + n2;
 }
 
+uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
+{
+    return fifo8_peekpop_buf(fifo, dest, destlen, true);
+}
+
 void fifo8_drop(Fifo8 *fifo, uint32_t len)
 {
     len -= fifo8_pop_buf(fifo, NULL, len);
-- 
2.39.2

[PATCH 6/9] fifo8: honour do_pop argument in fifo8_peekpop_buf()

[Mark Cave-Ayland]

Pass the do_pop value from fifo8_peekpop_buf() to fifo8_peekpop_bufptr() to
allow peeks to the FIFO buffer, including adjusting the skip parameter to
handle the case where the internal FIFO buffer wraps around.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 util/fifo8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/fifo8.c b/util/fifo8.c
index 5453cbc1b0..1031ffbe7e 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -117,7 +117,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
     }
 
     len = destlen;
-    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
+    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, do_pop);
     if (dest) {
         memcpy(dest, buf, n1);
     }
@@ -126,7 +126,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
     len -= n1;
     len = MIN(len, fifo8_num_used(fifo));
     if (len) {
-        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
+        buf = fifo8_peekpop_bufptr(fifo, len, do_pop ? 0 : n1, &n2, do_pop);
         if (dest) {
             memcpy(&dest[n1], buf, n2);
         }
-- 
2.39.2

[PATCH 7/9] fifo8: add fifo8_peek_buf() function

[Mark Cave-Ayland]

This is a wrapper function around fifo8_peekpop_buf() that allows the caller to
peek into FIFO, including handling the case where there is a wraparound of the
internal FIFO buffer.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 include/qemu/fifo8.h | 14 ++++++++++++++
 util/fifo8.c         |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
index d1d06754d8..d09984b146 100644
--- a/include/qemu/fifo8.h
+++ b/include/qemu/fifo8.h
@@ -76,6 +76,20 @@ uint8_t fifo8_pop(Fifo8 *fifo);
  */
 uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
 
+/**
+ * fifo8_peek_buf:
+ * @fifo: FIFO to read from
+ * @dest: the buffer to write the data into (can be NULL)
+ * @destlen: size of @dest and maximum number of bytes to peek
+ *
+ * Peek a number of elements from the FIFO up to a maximum of @destlen.
+ * The peeked data is copied into the @dest buffer.
+ * Care is taken when the data wraps around in the ring buffer.
+ *
+ * Returns: number of bytes peeked.
+ */
+uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
+
 /**
  * fifo8_pop_bufptr:
  * @fifo: FIFO to pop from
diff --git a/util/fifo8.c b/util/fifo8.c
index 1031ffbe7e..a8f5cea158 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -140,6 +140,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
     return fifo8_peekpop_buf(fifo, dest, destlen, true);
 }
 
+uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
+{
+    return fifo8_peekpop_buf(fifo, dest, destlen, false);
+}
+
 void fifo8_drop(Fifo8 *fifo, uint32_t len)
 {
     len -= fifo8_pop_buf(fifo, NULL, len);
-- 
2.39.2

[PATCH 8/9] fifo8: introduce fifo8_peek() function

[Mark Cave-Ayland]

This allows uses to peek the byte at the current head of the FIFO.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 include/qemu/fifo8.h | 11 +++++++++++
 util/fifo8.c         |  6 ++++++
 2 files changed, 17 insertions(+)

diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
index d09984b146..4f768d4ee3 100644
--- a/include/qemu/fifo8.h
+++ b/include/qemu/fifo8.h
@@ -62,6 +62,17 @@ void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
  */
 uint8_t fifo8_pop(Fifo8 *fifo);
 
+/**
+ * fifo8_peek:
+ * @fifo: fifo to peek from
+ *
+ * Peek the data byte at the current head of the FIFO. Clients are responsible
+ * for checking for emptyness using fifo8_is_empty().
+ *
+ * Returns: The peeked data byte.
+ */
+uint8_t fifo8_peek(Fifo8 *fifo);
+
 /**
  * fifo8_pop_buf:
  * @fifo: FIFO to pop from
diff --git a/util/fifo8.c b/util/fifo8.c
index a8f5cea158..a26da66ad2 100644
--- a/util/fifo8.c
+++ b/util/fifo8.c
@@ -71,6 +71,12 @@ uint8_t fifo8_pop(Fifo8 *fifo)
     return ret;
 }
 
+uint8_t fifo8_peek(Fifo8 *fifo)
+{
+    assert(fifo->num > 0);
+    return fifo->data[fifo->head];
+}
+
 static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
                                            uint32_t skip, uint32_t *numptr,
                                            bool do_pop)
-- 
2.39.2

[PATCH 9/9] tests/unit: add test-fifo unit test

[Mark Cave-Ayland]

This tests the Fifo8 implementation for basic operations as well as testing for
the correct *_bufptr() including handling wraparound of the internal FIFO buffer.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 tests/unit/meson.build |   1 +
 tests/unit/test-fifo.c | 256 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 257 insertions(+)
 create mode 100644 tests/unit/test-fifo.c

diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 490ab8182d..89f9633cd6 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -47,6 +47,7 @@ tests = {
   'test-logging': [],
   'test-qapi-util': [],
   'test-interval-tree': [],
+  'test-fifo': [],
 }
 
 if have_system or have_tools
diff --git a/tests/unit/test-fifo.c b/tests/unit/test-fifo.c
new file mode 100644
index 0000000000..1e54cde871
--- /dev/null
+++ b/tests/unit/test-fifo.c
@@ -0,0 +1,256 @@
+/*
+ * Fifo8 tests
+ *
+ * Copyright 2024 Mark Cave-Ayland
+ *
+ * Authors:
+ *  Mark Cave-Ayland    <mark.cave-ayland@ilande.co.uk>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "migration/vmstate.h"
+#include "qemu/fifo8.h"
+
+const VMStateInfo vmstate_info_uint32;
+const VMStateInfo vmstate_info_buffer;
+
+
+static void test_fifo8_pop_bufptr_wrap(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
+    const uint8_t *buf;
+    uint32_t count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
+    buf = fifo8_pop_bufptr(&fifo, 2, &count);
+    g_assert(count == 2);
+    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
+
+    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
+    buf = fifo8_pop_bufptr(&fifo, 8, &count);
+    g_assert(count == 6);
+    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
+             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
+
+    g_assert(fifo8_num_used(&fifo) == 2);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_pop_bufptr(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
+    const uint8_t *buf;
+    uint32_t count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in, sizeof(data_in));
+    buf = fifo8_pop_bufptr(&fifo, 2, &count);
+    g_assert(count == 2);
+    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
+
+    g_assert(fifo8_num_used(&fifo) == 2);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_peek_bufptr_wrap(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
+    const uint8_t *buf;
+    uint32_t count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
+    buf = fifo8_peek_bufptr(&fifo, 2, &count);
+    g_assert(count == 2);
+    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
+
+    buf = fifo8_pop_bufptr(&fifo, 2, &count);
+    g_assert(count == 2);
+    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
+    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
+
+    buf = fifo8_peek_bufptr(&fifo, 8, &count);
+    g_assert(count == 6);
+    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
+             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
+
+    g_assert(fifo8_num_used(&fifo) == 8);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_peek_bufptr(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
+    const uint8_t *buf;
+    uint32_t count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in, sizeof(data_in));
+    buf = fifo8_peek_bufptr(&fifo, 2, &count);
+    g_assert(count == 2);
+    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
+
+    g_assert(fifo8_num_used(&fifo) == 4);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_pop_buf_wrap(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_out[4];
+    int count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
+    fifo8_pop_buf(&fifo, NULL, 4);
+
+    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
+    count = fifo8_pop_buf(&fifo, NULL, 4);
+    g_assert(count == 4);
+    count = fifo8_pop_buf(&fifo, data_out, 4);
+    g_assert(count == 4);
+    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
+             data_out[2] == 0x3 && data_out[3] == 0x4);
+
+    g_assert(fifo8_num_used(&fifo) == 0);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_pop_buf(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8 };
+    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
+    int count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in, sizeof(data_in));
+    count = fifo8_pop_buf(&fifo, NULL, 4);
+    g_assert(count == 4);
+    count = fifo8_pop_buf(&fifo, data_out, 4);
+    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
+             data_out[2] == 0x7 && data_out[3] == 0x8);
+
+    g_assert(fifo8_num_used(&fifo) == 0);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_peek_buf_wrap(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_out[4];
+    int count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
+    fifo8_pop_buf(&fifo, NULL, 4);
+
+    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
+    count = fifo8_peek_buf(&fifo, NULL, 4);
+    g_assert(count == 4);
+    count = fifo8_peek_buf(&fifo, data_out, 4);
+    g_assert(count == 4);
+    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
+             data_out[2] == 0x7 && data_out[3] == 0x8);
+
+    g_assert(fifo8_num_used(&fifo) == 8);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_peek_buf(void)
+{
+    Fifo8 fifo;
+    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
+    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
+    int count;
+
+    fifo8_create(&fifo, 8);
+
+    fifo8_push_all(&fifo, data_in, sizeof(data_in));
+    count = fifo8_peek_buf(&fifo, NULL, 4);
+    g_assert(count == 4);
+    g_assert(data_out[0] == 0xff && data_out[1] == 0xff &&
+             data_out[2] == 0xff && data_out[3] == 0xff);
+
+    count = fifo8_peek_buf(&fifo, data_out, 4);
+    g_assert(count == 4);
+    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
+             data_out[2] == 0x3 && data_out[3] == 0x4);
+
+    g_assert(fifo8_num_used(&fifo) == 4);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_peek(void)
+{
+    Fifo8 fifo;
+    uint8_t c;
+
+    fifo8_create(&fifo, 8);
+    fifo8_push(&fifo, 0x1);
+    fifo8_push(&fifo, 0x2);
+
+    c = fifo8_peek(&fifo);
+    g_assert(c == 0x1);
+    fifo8_pop(&fifo);
+    c = fifo8_peek(&fifo);
+    g_assert(c == 0x2);
+
+    g_assert(fifo8_num_used(&fifo) == 1);
+    fifo8_destroy(&fifo);
+}
+
+static void test_fifo8_pushpop(void)
+{
+    Fifo8 fifo;
+    uint8_t c;
+
+    fifo8_create(&fifo, 8);
+    fifo8_push(&fifo, 0x1);
+    fifo8_push(&fifo, 0x2);
+
+    c = fifo8_pop(&fifo);
+    g_assert(c == 0x1);
+    c = fifo8_pop(&fifo);
+    g_assert(c == 0x2);
+
+    g_assert(fifo8_num_used(&fifo) == 0);
+    fifo8_destroy(&fifo);
+}
+
+int main(int argc, char *argv[])
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/fifo8/pushpop", test_fifo8_pushpop);
+    g_test_add_func("/fifo8/peek", test_fifo8_peek);
+    g_test_add_func("/fifo8/peek_buf", test_fifo8_peek_buf);
+    g_test_add_func("/fifo8/peek_buf_wrap", test_fifo8_peek_buf_wrap);
+    g_test_add_func("/fifo8/pop_buf", test_fifo8_pop_buf);
+    g_test_add_func("/fifo8/pop_buf_wrap", test_fifo8_pop_buf_wrap);
+    g_test_add_func("/fifo8/peek_bufptr", test_fifo8_peek_bufptr);
+    g_test_add_func("/fifo8/peek_bufptr_wrap", test_fifo8_peek_bufptr_wrap);
+    g_test_add_func("/fifo8/pop_bufptr", test_fifo8_pop_bufptr);
+    g_test_add_func("/fifo8/pop_bufptr_wrap", test_fifo8_pop_bufptr_wrap);
+    return g_test_run();
+}
-- 
2.39.2

Re: [PATCH 1/9] fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This is to emphasise that the function returns a pointer to the internal FIFO
> buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 1ffa19d900..61bce9d9a0 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -71,8 +71,8 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>      return ret;
>  }
>
> -static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
> -                                        uint32_t *numptr, bool do_pop)
> +static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
> +                                           uint32_t *numptr, bool do_pop)
>  {
>      uint8_t *ret;
>      uint32_t num;
> @@ -94,12 +94,12 @@ static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
>
>  const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_buf(fifo, max, numptr, false);
> +    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
>  }
>
>  const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_buf(fifo, max, numptr, true);
> +    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
>  }
>
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> --
> 2.39.2
>
>

Re: [PATCH 2/9] fifo8: introduce head variable for fifo8_peekpop_bufptr()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Rather than operate on fifo->head directly, introduce a new head variable which is
> set to the value of fifo->head and use it instead. This is to allow future
> adjustment of the head position within the internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 61bce9d9a0..5faa814a6e 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -75,11 +75,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>                                             uint32_t *numptr, bool do_pop)
>  {
>      uint8_t *ret;
> -    uint32_t num;
> +    uint32_t num, head;
>
>      assert(max > 0 && max <= fifo->num);
> -    num = MIN(fifo->capacity - fifo->head, max);
> -    ret = &fifo->data[fifo->head];
> +    head = fifo->head;
> +    num = MIN(fifo->capacity - head, max);
> +    ret = &fifo->data[head];
>
>      if (do_pop) {
>          fifo->head += num;
> --
> 2.39.2
>
>

Re: [PATCH 3/9] fifo8: add skip parameter to fifo8_peekpop_bufptr()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:24 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The skip parameter specifies the number of bytes to be skipped from the current
> FIFO head before the peek or pop operation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 5faa814a6e..62d6430b05 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -72,18 +72,20 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>  }
>
>  static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
> -                                           uint32_t *numptr, bool do_pop)
> +                                           uint32_t skip, uint32_t *numptr,
> +                                           bool do_pop)
>  {
>      uint8_t *ret;
>      uint32_t num, head;
>
>      assert(max > 0 && max <= fifo->num);
> -    head = fifo->head;
> +    assert(skip <= fifo->num);
> +    head = (fifo->head + skip) % fifo->capacity;
>      num = MIN(fifo->capacity - head, max);
>      ret = &fifo->data[head];
>
>      if (do_pop) {
> -        fifo->head += num;
> +        fifo->head = head + num;
>          fifo->head %= fifo->capacity;
>          fifo->num -= num;
>      }
> @@ -95,12 +97,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>
>  const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
> +    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, false);
>  }
>
>  const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
> +    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
>  }
>
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> --
> 2.39.2
>
>

Re: [PATCH 4/9] fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in fifo8_pop_buf()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The upcoming peek functionality will require passing a non-zero value to
> fifo8_peekpop_bufptr().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 62d6430b05..efe0117b1f 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -116,7 +116,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      }
>
>      len = destlen;
> -    buf = fifo8_pop_bufptr(fifo, len, &n1);
> +    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
>      if (dest) {
>          memcpy(dest, buf, n1);
>      }
> @@ -125,7 +125,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      len -= n1;
>      len = MIN(len, fifo8_num_used(fifo));
>      if (len) {
> -        buf = fifo8_pop_bufptr(fifo, len, &n2);
> +        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
>          if (dest) {
>              memcpy(&dest[n1], buf, n2);
>          }
> --
> 2.39.2
>
>

Re: [PATCH 5/9] fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The fifo8_pop_buf() function will soon also be used for peek operations, so rename
> the function accordingly. Create a new fifo8_pop_buf() wrapper function that can
> be used by existing callers.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index efe0117b1f..5453cbc1b0 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -105,7 +105,8 @@ const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>      return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
>  }
>
> -uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
> +                                  bool do_pop)
>  {
>      const uint8_t *buf;
>      uint32_t n1, n2 = 0;
> @@ -134,6 +135,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      return n1 + n2;
>  }
>
> +uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +{
> +    return fifo8_peekpop_buf(fifo, dest, destlen, true);
> +}
> +
>  void fifo8_drop(Fifo8 *fifo, uint32_t len)
>  {
>      len -= fifo8_pop_buf(fifo, NULL, len);
> --
> 2.39.2
>
>

Re: [PATCH 6/9] fifo8: honour do_pop argument in fifo8_peekpop_buf()

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Pass the do_pop value from fifo8_peekpop_buf() to fifo8_peekpop_bufptr() to
> allow peeks to the FIFO buffer, including adjusting the skip parameter to
> handle the case where the internal FIFO buffer wraps around.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  util/fifo8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 5453cbc1b0..1031ffbe7e 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -117,7 +117,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
>      }
>
>      len = destlen;
> -    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
> +    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, do_pop);
>      if (dest) {
>          memcpy(dest, buf, n1);
>      }
> @@ -126,7 +126,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
>      len -= n1;
>      len = MIN(len, fifo8_num_used(fifo));
>      if (len) {
> -        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
> +        buf = fifo8_peekpop_bufptr(fifo, len, do_pop ? 0 : n1, &n2, do_pop);
>          if (dest) {
>              memcpy(&dest[n1], buf, n2);
>          }
> --
> 2.39.2
>
>

Re: [PATCH 7/9] fifo8: add fifo8_peek_buf() function

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This is a wrapper function around fifo8_peekpop_buf() that allows the caller to
> peek into FIFO, including handling the case where there is a wraparound of the
> internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/qemu/fifo8.h | 14 ++++++++++++++
>  util/fifo8.c         |  5 +++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> index d1d06754d8..d09984b146 100644
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo8.h
> @@ -76,6 +76,20 @@ uint8_t fifo8_pop(Fifo8 *fifo);
>   */
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
>
> +/**
> + * fifo8_peek_buf:
> + * @fifo: FIFO to read from
> + * @dest: the buffer to write the data into (can be NULL)
> + * @destlen: size of @dest and maximum number of bytes to peek
> + *
> + * Peek a number of elements from the FIFO up to a maximum of @destlen.
> + * The peeked data is copied into the @dest buffer.
> + * Care is taken when the data wraps around in the ring buffer.
> + *
> + * Returns: number of bytes peeked.
> + */
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
> +
>  /**
>   * fifo8_pop_bufptr:
>   * @fifo: FIFO to pop from
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 1031ffbe7e..a8f5cea158 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -140,6 +140,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      return fifo8_peekpop_buf(fifo, dest, destlen, true);
>  }
>
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +{
> +    return fifo8_peekpop_buf(fifo, dest, destlen, false);
> +}
> +
>  void fifo8_drop(Fifo8 *fifo, uint32_t len)
>  {
>      len -= fifo8_pop_buf(fifo, NULL, len);
> --
> 2.39.2
>
>

Re: [PATCH 8/9] fifo8: introduce fifo8_peek() function

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:26 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This allows uses to peek the byte at the current head of the FIFO.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/qemu/fifo8.h | 11 +++++++++++
>  util/fifo8.c         |  6 ++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> index d09984b146..4f768d4ee3 100644
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo8.h
> @@ -62,6 +62,17 @@ void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
>   */
>  uint8_t fifo8_pop(Fifo8 *fifo);
>
> +/**
> + * fifo8_peek:
> + * @fifo: fifo to peek from
> + *
> + * Peek the data byte at the current head of the FIFO. Clients are responsible
> + * for checking for emptyness using fifo8_is_empty().
> + *
> + * Returns: The peeked data byte.
> + */
> +uint8_t fifo8_peek(Fifo8 *fifo);
> +
>  /**
>   * fifo8_pop_buf:
>   * @fifo: FIFO to pop from
> diff --git a/util/fifo8.c b/util/fifo8.c
> index a8f5cea158..a26da66ad2 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -71,6 +71,12 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>      return ret;
>  }
>
> +uint8_t fifo8_peek(Fifo8 *fifo)
> +{
> +    assert(fifo->num > 0);
> +    return fifo->data[fifo->head];
> +}
> +
>  static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>                                             uint32_t skip, uint32_t *numptr,
>                                             bool do_pop)
> --
> 2.39.2
>
>

Re: [PATCH 9/9] tests/unit: add test-fifo unit test

[Alistair Francis]

On Wed, Aug 28, 2024 at 10:25 PM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This tests the Fifo8 implementation for basic operations as well as testing for
> the correct *_bufptr() including handling wraparound of the internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  tests/unit/meson.build |   1 +
>  tests/unit/test-fifo.c | 256 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 257 insertions(+)
>  create mode 100644 tests/unit/test-fifo.c
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 490ab8182d..89f9633cd6 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -47,6 +47,7 @@ tests = {
>    'test-logging': [],
>    'test-qapi-util': [],
>    'test-interval-tree': [],
> +  'test-fifo': [],
>  }
>
>  if have_system or have_tools
> diff --git a/tests/unit/test-fifo.c b/tests/unit/test-fifo.c
> new file mode 100644
> index 0000000000..1e54cde871
> --- /dev/null
> +++ b/tests/unit/test-fifo.c
> @@ -0,0 +1,256 @@
> +/*
> + * Fifo8 tests
> + *
> + * Copyright 2024 Mark Cave-Ayland
> + *
> + * Authors:
> + *  Mark Cave-Ayland    <mark.cave-ayland@ilande.co.uk>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "qemu/fifo8.h"
> +
> +const VMStateInfo vmstate_info_uint32;
> +const VMStateInfo vmstate_info_buffer;
> +
> +
> +static void test_fifo8_pop_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    buf = fifo8_pop_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +
> +    buf = fifo8_peek_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0xff && data_out[1] == 0xff &&
> +             data_out[2] == 0xff && data_out[3] == 0xff);
> +
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x1);
> +    fifo8_pop(&fifo);
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 1);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pushpop(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x1);
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    g_test_add_func("/fifo8/pushpop", test_fifo8_pushpop);
> +    g_test_add_func("/fifo8/peek", test_fifo8_peek);
> +    g_test_add_func("/fifo8/peek_buf", test_fifo8_peek_buf);
> +    g_test_add_func("/fifo8/peek_buf_wrap", test_fifo8_peek_buf_wrap);
> +    g_test_add_func("/fifo8/pop_buf", test_fifo8_pop_buf);
> +    g_test_add_func("/fifo8/pop_buf_wrap", test_fifo8_pop_buf_wrap);
> +    g_test_add_func("/fifo8/peek_bufptr", test_fifo8_peek_bufptr);
> +    g_test_add_func("/fifo8/peek_bufptr_wrap", test_fifo8_peek_bufptr_wrap);
> +    g_test_add_func("/fifo8/pop_bufptr", test_fifo8_pop_bufptr);
> +    g_test_add_func("/fifo8/pop_bufptr_wrap", test_fifo8_pop_bufptr_wrap);
> +    return g_test_run();
> +}
> --
> 2.39.2
>
>

Re: [PATCH 1/9] fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This is to emphasise that the function returns a pointer to the internal FIFO
> buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  util/fifo8.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 1ffa19d900..61bce9d9a0 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -71,8 +71,8 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>      return ret;
>  }
>
> -static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
> -                                        uint32_t *numptr, bool do_pop)
> +static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
> +                                           uint32_t *numptr, bool do_pop)
>  {
>      uint8_t *ret;
>      uint32_t num;
> @@ -94,12 +94,12 @@ static const uint8_t *fifo8_peekpop_buf(Fifo8 *fifo, uint32_t max,
>
>  const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_buf(fifo, max, numptr, false);
> +    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
>  }
>
>  const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_buf(fifo, max, numptr, true);
> +    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
>  }
>
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> --
> 2.39.2
>

Re: [PATCH 2/9] fifo8: introduce head variable for fifo8_peekpop_bufptr()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Rather than operate on fifo->head directly, introduce a new head variable which is
> set to the value of fifo->head and use it instead. This is to allow future
> adjustment of the head position within the internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  util/fifo8.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 61bce9d9a0..5faa814a6e 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -75,11 +75,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>                                             uint32_t *numptr, bool do_pop)
>  {
>      uint8_t *ret;
> -    uint32_t num;
> +    uint32_t num, head;
>
>      assert(max > 0 && max <= fifo->num);
> -    num = MIN(fifo->capacity - fifo->head, max);
> -    ret = &fifo->data[fifo->head];
> +    head = fifo->head;
> +    num = MIN(fifo->capacity - head, max);
> +    ret = &fifo->data[head];
>
>      if (do_pop) {
>          fifo->head += num;
> --
> 2.39.2
>

Re: [PATCH 3/9] fifo8: add skip parameter to fifo8_peekpop_bufptr()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The skip parameter specifies the number of bytes to be skipped from the current
> FIFO head before the peek or pop operation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  util/fifo8.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 5faa814a6e..62d6430b05 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -72,18 +72,20 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>  }
>
>  static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
> -                                           uint32_t *numptr, bool do_pop)
> +                                           uint32_t skip, uint32_t *numptr,
> +                                           bool do_pop)
>  {
>      uint8_t *ret;
>      uint32_t num, head;
>
>      assert(max > 0 && max <= fifo->num);
> -    head = fifo->head;
> +    assert(skip <= fifo->num);
> +    head = (fifo->head + skip) % fifo->capacity;
>      num = MIN(fifo->capacity - head, max);
>      ret = &fifo->data[head];
>
>      if (do_pop) {
> -        fifo->head += num;
> +        fifo->head = head + num;
>          fifo->head %= fifo->capacity;
>          fifo->num -= num;
>      }
> @@ -95,12 +97,12 @@ static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>
>  const uint8_t *fifo8_peek_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_bufptr(fifo, max, numptr, false);
> +    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, false);
>  }
>
>  const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>  {
> -    return fifo8_peekpop_bufptr(fifo, max, numptr, true);
> +    return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
>  }
>
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> --
> 2.39.2
>

Re: [PATCH 4/9] fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in fifo8_pop_buf()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The upcoming peek functionality will require passing a non-zero value to
> fifo8_peekpop_bufptr().
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  util/fifo8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 62d6430b05..efe0117b1f 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -116,7 +116,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      }
>
>      len = destlen;
> -    buf = fifo8_pop_bufptr(fifo, len, &n1);
> +    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
>      if (dest) {
>          memcpy(dest, buf, n1);
>      }
> @@ -125,7 +125,7 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      len -= n1;
>      len = MIN(len, fifo8_num_used(fifo));
>      if (len) {
> -        buf = fifo8_pop_bufptr(fifo, len, &n2);
> +        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
>          if (dest) {
>              memcpy(&dest[n1], buf, n2);
>          }
> --
> 2.39.2
>

Re: [PATCH 5/9] fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The fifo8_pop_buf() function will soon also be used for peek operations, so rename
> the function accordingly. Create a new fifo8_pop_buf() wrapper function that can
> be used by existing callers.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  util/fifo8.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index efe0117b1f..5453cbc1b0 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -105,7 +105,8 @@ const uint8_t *fifo8_pop_bufptr(Fifo8 *fifo, uint32_t max, uint32_t *numptr)
>      return fifo8_peekpop_bufptr(fifo, max, 0, numptr, true);
>  }
>
> -uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
> +                                  bool do_pop)
>  {
>      const uint8_t *buf;
>      uint32_t n1, n2 = 0;
> @@ -134,6 +135,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      return n1 + n2;
>  }
>
> +uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +{
> +    return fifo8_peekpop_buf(fifo, dest, destlen, true);
> +}
> +
>  void fifo8_drop(Fifo8 *fifo, uint32_t len)
>  {
>      len -= fifo8_pop_buf(fifo, NULL, len);
> --
> 2.39.2
>

Re: [PATCH 6/9] fifo8: honour do_pop argument in fifo8_peekpop_buf()

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Pass the do_pop value from fifo8_peekpop_buf() to fifo8_peekpop_bufptr() to
> allow peeks to the FIFO buffer, including adjusting the skip parameter to
> handle the case where the internal FIFO buffer wraps around.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>


> ---
>  util/fifo8.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 5453cbc1b0..1031ffbe7e 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -117,7 +117,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
>      }
>
>      len = destlen;
> -    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, true);
> +    buf = fifo8_peekpop_bufptr(fifo, len, 0, &n1, do_pop);
>      if (dest) {
>          memcpy(dest, buf, n1);
>      }
> @@ -126,7 +126,7 @@ static uint32_t fifo8_peekpop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen,
>      len -= n1;
>      len = MIN(len, fifo8_num_used(fifo));
>      if (len) {
> -        buf = fifo8_peekpop_bufptr(fifo, len, 0, &n2, true);
> +        buf = fifo8_peekpop_bufptr(fifo, len, do_pop ? 0 : n1, &n2, do_pop);
>          if (dest) {
>              memcpy(&dest[n1], buf, n2);
>          }
> --
> 2.39.2
>

Re: [PATCH 7/9] fifo8: add fifo8_peek_buf() function

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This is a wrapper function around fifo8_peekpop_buf() that allows the caller to
> peek into FIFO, including handling the case where there is a wraparound of the
> internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  include/qemu/fifo8.h | 14 ++++++++++++++
>  util/fifo8.c         |  5 +++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> index d1d06754d8..d09984b146 100644
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo8.h
> @@ -76,6 +76,20 @@ uint8_t fifo8_pop(Fifo8 *fifo);
>   */
>  uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
>
> +/**
> + * fifo8_peek_buf:
> + * @fifo: FIFO to read from
> + * @dest: the buffer to write the data into (can be NULL)
> + * @destlen: size of @dest and maximum number of bytes to peek
> + *
> + * Peek a number of elements from the FIFO up to a maximum of @destlen.
> + * The peeked data is copied into the @dest buffer.
> + * Care is taken when the data wraps around in the ring buffer.
> + *
> + * Returns: number of bytes peeked.
> + */
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
> +
>  /**
>   * fifo8_pop_bufptr:
>   * @fifo: FIFO to pop from
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 1031ffbe7e..a8f5cea158 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -140,6 +140,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>      return fifo8_peekpop_buf(fifo, dest, destlen, true);
>  }
>
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +{
> +    return fifo8_peekpop_buf(fifo, dest, destlen, false);
> +}
> +
>  void fifo8_drop(Fifo8 *fifo, uint32_t len)
>  {
>      len -= fifo8_pop_buf(fifo, NULL, len);
> --
> 2.39.2
>

Re: [PATCH 8/9] fifo8: introduce fifo8_peek() function

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This allows uses to peek the byte at the current head of the FIFO.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  include/qemu/fifo8.h | 11 +++++++++++
>  util/fifo8.c         |  6 ++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> index d09984b146..4f768d4ee3 100644
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo8.h
> @@ -62,6 +62,17 @@ void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num);
>   */
>  uint8_t fifo8_pop(Fifo8 *fifo);
>
> +/**
> + * fifo8_peek:
> + * @fifo: fifo to peek from
> + *
> + * Peek the data byte at the current head of the FIFO. Clients are responsible
> + * for checking for emptyness using fifo8_is_empty().
> + *
> + * Returns: The peeked data byte.
> + */
> +uint8_t fifo8_peek(Fifo8 *fifo);
> +
>  /**
>   * fifo8_pop_buf:
>   * @fifo: FIFO to pop from
> diff --git a/util/fifo8.c b/util/fifo8.c
> index a8f5cea158..a26da66ad2 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -71,6 +71,12 @@ uint8_t fifo8_pop(Fifo8 *fifo)
>      return ret;
>  }
>
> +uint8_t fifo8_peek(Fifo8 *fifo)
> +{
> +    assert(fifo->num > 0);
> +    return fifo->data[fifo->head];
> +}
> +
>  static const uint8_t *fifo8_peekpop_bufptr(Fifo8 *fifo, uint32_t max,
>                                             uint32_t skip, uint32_t *numptr,
>                                             bool do_pop)
> --
> 2.39.2
>

Re: [PATCH 9/9] tests/unit: add test-fifo unit test

[Octavian Purdila]

On Wed, Aug 28, 2024 at 5:23 AM Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This tests the Fifo8 implementation for basic operations as well as testing for
> the correct *_bufptr() including handling wraparound of the internal FIFO buffer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Reviewed-by: Octavian Purdila <tavip@google.com>

> ---
>  tests/unit/meson.build |   1 +
>  tests/unit/test-fifo.c | 256 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 257 insertions(+)
>  create mode 100644 tests/unit/test-fifo.c
>
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 490ab8182d..89f9633cd6 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -47,6 +47,7 @@ tests = {
>    'test-logging': [],
>    'test-qapi-util': [],
>    'test-interval-tree': [],
> +  'test-fifo': [],
>  }
>
>  if have_system or have_tools
> diff --git a/tests/unit/test-fifo.c b/tests/unit/test-fifo.c
> new file mode 100644
> index 0000000000..1e54cde871
> --- /dev/null
> +++ b/tests/unit/test-fifo.c
> @@ -0,0 +1,256 @@
> +/*
> + * Fifo8 tests
> + *
> + * Copyright 2024 Mark Cave-Ayland
> + *
> + * Authors:
> + *  Mark Cave-Ayland    <mark.cave-ayland@ilande.co.uk>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "qemu/fifo8.h"
> +
> +const VMStateInfo vmstate_info_uint32;
> +const VMStateInfo vmstate_info_buffer;
> +
> +
> +static void test_fifo8_pop_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    buf = fifo8_pop_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +
> +    buf = fifo8_peek_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +with known values.

> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];

Initialize data_out.

> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];

Initialize data_out.

> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0xff && data_out[1] == 0xff &&
> +             data_out[2] == 0xff && data_out[3] == 0xff);
> +
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x1);
> +    fifo8_pop(&fifo);
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 1);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pushpop(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x1);
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    g_test_add_func("/fifo8/pushpop", test_fifo8_pushpop);
> +    g_test_add_func("/fifo8/peek", test_fifo8_peek);
> +    g_test_add_func("/fifo8/peek_buf", test_fifo8_peek_buf);
> +    g_test_add_func("/fifo8/peek_buf_wrap", test_fifo8_peek_buf_wrap);
> +    g_test_add_func("/fifo8/pop_buf", test_fifo8_pop_buf);
> +    g_test_add_func("/fifo8/pop_buf_wrap", test_fifo8_pop_buf_wrap);
> +    g_test_add_func("/fifo8/peek_bufptr", test_fifo8_peek_bufptr);
> +    g_test_add_func("/fifo8/peek_bufptr_wrap", test_fifo8_peek_bufptr_wrap);
> +    g_test_add_func("/fifo8/pop_bufptr", test_fifo8_pop_bufptr);
> +    g_test_add_func("/fifo8/pop_bufptr_wrap", test_fifo8_pop_bufptr_wrap);
> +    return g_test_run();
> +}
> --
> 2.39.2
>

Re: [PATCH 0/9] fifo8: add fifo8_peek(), fifo8_peek_buf() and tests

[Philippe Mathieu-Daudé]

Hi Mark,

On 28/8/24 14:22, Mark Cave-Ayland wrote:

> Mark Cave-Ayland (9):
>    fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()
>    fifo8: introduce head variable for fifo8_peekpop_bufptr()
>    fifo8: add skip parameter to fifo8_peekpop_bufptr()
>    fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in
>      fifo8_pop_buf()
>    fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()
>    fifo8: honour do_pop argument in fifo8_peekpop_buf()
>    fifo8: add fifo8_peek_buf() function
>    fifo8: introduce fifo8_peek() function
>    tests/unit: add test-fifo unit test

For this series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

I'm OK to queue it but took some notes while reviewing:
https://lore.kernel.org/qemu-devel/20240906131217.78159-1-philmd@linaro.org/
If you can have a look, I'll queue both together.

Thanks!

Phil.

Re: [PATCH 7/9] fifo8: add fifo8_peek_buf() function

[Mark Cave-Ayland]

On 28/08/2024 13:22, Mark Cave-Ayland wrote:

> This is a wrapper function around fifo8_peekpop_buf() that allows the caller to
> peek into FIFO, including handling the case where there is a wraparound of the

peek into the FIFO

Looks like I missed a "the" out in the commit message above.

> internal FIFO buffer.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   include/qemu/fifo8.h | 14 ++++++++++++++
>   util/fifo8.c         |  5 +++++
>   2 files changed, 19 insertions(+)
> 
> diff --git a/include/qemu/fifo8.h b/include/qemu/fifo8.h
> index d1d06754d8..d09984b146 100644
> --- a/include/qemu/fifo8.h
> +++ b/include/qemu/fifo8.h
> @@ -76,6 +76,20 @@ uint8_t fifo8_pop(Fifo8 *fifo);
>    */
>   uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
>   
> +/**
> + * fifo8_peek_buf:
> + * @fifo: FIFO to read from
> + * @dest: the buffer to write the data into (can be NULL)
> + * @destlen: size of @dest and maximum number of bytes to peek
> + *
> + * Peek a number of elements from the FIFO up to a maximum of @destlen.
> + * The peeked data is copied into the @dest buffer.
> + * Care is taken when the data wraps around in the ring buffer.
> + *
> + * Returns: number of bytes peeked.
> + */
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen);
> +
>   /**
>    * fifo8_pop_bufptr:
>    * @fifo: FIFO to pop from
> diff --git a/util/fifo8.c b/util/fifo8.c
> index 1031ffbe7e..a8f5cea158 100644
> --- a/util/fifo8.c
> +++ b/util/fifo8.c
> @@ -140,6 +140,11 @@ uint32_t fifo8_pop_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
>       return fifo8_peekpop_buf(fifo, dest, destlen, true);
>   }
>   
> +uint32_t fifo8_peek_buf(Fifo8 *fifo, uint8_t *dest, uint32_t destlen)
> +{
> +    return fifo8_peekpop_buf(fifo, dest, destlen, false);
> +}
> +
>   void fifo8_drop(Fifo8 *fifo, uint32_t len)
>   {
>       len -= fifo8_pop_buf(fifo, NULL, len);


ATB,

Mark.

Re: [PATCH 9/9] tests/unit: add test-fifo unit test

[Mark Cave-Ayland]

On 28/08/2024 13:22, Mark Cave-Ayland wrote:

> This tests the Fifo8 implementation for basic operations as well as testing for
> the correct *_bufptr() including handling wraparound of the internal FIFO buffer.

Hmmm this doesn't quite read correctly either - I think perhaps something like:

This tests the Fifo8 implementation basic operations as well as testing the 
*_bufptr() in-place buffer functions and the newer *_buf() functions that also handle 
wraparound of the internal FIFO buffer.

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   tests/unit/meson.build |   1 +
>   tests/unit/test-fifo.c | 256 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 257 insertions(+)
>   create mode 100644 tests/unit/test-fifo.c
> 
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 490ab8182d..89f9633cd6 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -47,6 +47,7 @@ tests = {
>     'test-logging': [],
>     'test-qapi-util': [],
>     'test-interval-tree': [],
> +  'test-fifo': [],
>   }
>   
>   if have_system or have_tools
> diff --git a/tests/unit/test-fifo.c b/tests/unit/test-fifo.c
> new file mode 100644
> index 0000000000..1e54cde871
> --- /dev/null
> +++ b/tests/unit/test-fifo.c
> @@ -0,0 +1,256 @@
> +/*
> + * Fifo8 tests
> + *
> + * Copyright 2024 Mark Cave-Ayland
> + *
> + * Authors:
> + *  Mark Cave-Ayland    <mark.cave-ayland@ilande.co.uk>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2 or later.
> + * See the COPYING.LIB file in the top-level directory.

Regarding your comment on using SPDX is it also worth replacing the above two lines with:

     * SPDX-License-Identifier: GPL-2.0-or-later

?

> + */
> +
> +#include "qemu/osdep.h"
> +#include "migration/vmstate.h"
> +#include "qemu/fifo8.h"
> +
> +const VMStateInfo vmstate_info_uint32;
> +const VMStateInfo vmstate_info_buffer;
> +
> +
> +static void test_fifo8_pop_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    buf = fifo8_pop_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 2);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    buf = fifo8_pop_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +
> +    buf = fifo8_peek_bufptr(&fifo, 8, &count);
> +    g_assert(count == 6);
> +    g_assert(buf[0] == 0x3 && buf[1] == 0x4 && buf[2] == 0x5 &&
> +             buf[3] == 0x6 && buf[4] == 0x7 && buf[5] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_bufptr(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    const uint8_t *buf;
> +    uint32_t count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    buf = fifo8_peek_bufptr(&fifo, 2, &count);
> +    g_assert(count == 2);
> +    g_assert(buf[0] == 0x1 && buf[1] == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pop_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_pop_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_pop_buf(&fifo, data_out, 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf_wrap(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in1[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_in2[] = { 0x5, 0x6, 0x7, 0x8, 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[4];
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in1, sizeof(data_in1));
> +    fifo8_pop_buf(&fifo, NULL, 4);
> +
> +    fifo8_push_all(&fifo, data_in2, sizeof(data_in2));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x5 && data_out[1] == 0x6 &&
> +             data_out[2] == 0x7 && data_out[3] == 0x8);
> +
> +    g_assert(fifo8_num_used(&fifo) == 8);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek_buf(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t data_in[] = { 0x1, 0x2, 0x3, 0x4 };
> +    uint8_t data_out[] = { 0xff, 0xff, 0xff, 0xff };
> +    int count;
> +
> +    fifo8_create(&fifo, 8);
> +
> +    fifo8_push_all(&fifo, data_in, sizeof(data_in));
> +    count = fifo8_peek_buf(&fifo, NULL, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0xff && data_out[1] == 0xff &&
> +             data_out[2] == 0xff && data_out[3] == 0xff);
> +
> +    count = fifo8_peek_buf(&fifo, data_out, 4);
> +    g_assert(count == 4);
> +    g_assert(data_out[0] == 0x1 && data_out[1] == 0x2 &&
> +             data_out[2] == 0x3 && data_out[3] == 0x4);
> +
> +    g_assert(fifo8_num_used(&fifo) == 4);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_peek(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x1);
> +    fifo8_pop(&fifo);
> +    c = fifo8_peek(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 1);
> +    fifo8_destroy(&fifo);
> +}
> +
> +static void test_fifo8_pushpop(void)
> +{
> +    Fifo8 fifo;
> +    uint8_t c;
> +
> +    fifo8_create(&fifo, 8);
> +    fifo8_push(&fifo, 0x1);
> +    fifo8_push(&fifo, 0x2);
> +
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x1);
> +    c = fifo8_pop(&fifo);
> +    g_assert(c == 0x2);
> +
> +    g_assert(fifo8_num_used(&fifo) == 0);
> +    fifo8_destroy(&fifo);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    g_test_add_func("/fifo8/pushpop", test_fifo8_pushpop);
> +    g_test_add_func("/fifo8/peek", test_fifo8_peek);
> +    g_test_add_func("/fifo8/peek_buf", test_fifo8_peek_buf);
> +    g_test_add_func("/fifo8/peek_buf_wrap", test_fifo8_peek_buf_wrap);
> +    g_test_add_func("/fifo8/pop_buf", test_fifo8_pop_buf);
> +    g_test_add_func("/fifo8/pop_buf_wrap", test_fifo8_pop_buf_wrap);
> +    g_test_add_func("/fifo8/peek_bufptr", test_fifo8_peek_bufptr);
> +    g_test_add_func("/fifo8/peek_bufptr_wrap", test_fifo8_peek_bufptr_wrap);
> +    g_test_add_func("/fifo8/pop_bufptr", test_fifo8_pop_bufptr);
> +    g_test_add_func("/fifo8/pop_bufptr_wrap", test_fifo8_pop_bufptr_wrap);
> +    return g_test_run();
> +}


ATB,

Mark.

Re: [PATCH 0/9] fifo8: add fifo8_peek(), fifo8_peek_buf() and tests

[Mark Cave-Ayland]

On 06/09/2024 14:14, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 28/8/24 14:22, Mark Cave-Ayland wrote:
> 
>> Mark Cave-Ayland (9):
>>    fifo8: rename fifo8_peekpop_buf() to fifo8_peekpop_bufptr()
>>    fifo8: introduce head variable for fifo8_peekpop_bufptr()
>>    fifo8: add skip parameter to fifo8_peekpop_bufptr()
>>    fifo8: replace fifo8_pop_bufptr() with fifo8_peekpop_bufptr() in
>>      fifo8_pop_buf()
>>    fifo8: rename fifo8_pop_buf() to fifo8_peekpop_buf()
>>    fifo8: honour do_pop argument in fifo8_peekpop_buf()
>>    fifo8: add fifo8_peek_buf() function
>>    fifo8: introduce fifo8_peek() function
>>    tests/unit: add test-fifo unit test
> 
> For this series:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> I'm OK to queue it but took some notes while reviewing:
> https://lore.kernel.org/qemu-devel/20240906131217.78159-1-philmd@linaro.org/
> If you can have a look, I'll queue both together.

Thanks Phil!

I've just spotted a few minor issues with the series which I've just replied to: 
would you like me to send a v2, or is it easier just for you to correct them yourself?


ATB,

Mark.

Re: [PATCH 0/9] fifo8: add fifo8_peek(), fifo8_peek_buf() and tests

[Philippe Mathieu-Daudé]

On 6/9/24 22:51, Mark Cave-Ayland wrote:
> On 06/09/2024 14:14, Philippe Mathieu-Daudé wrote:
> 
>> Hi Mark,

>> I'm OK to queue it but took some notes while reviewing:
>> https://lore.kernel.org/qemu-devel/20240906131217.78159-1-philmd@linaro.org/
>> If you can have a look, I'll queue both together.
> 
> Thanks Phil!
> 
> I've just spotted a few minor issues with the series which I've just 
> replied to: would you like me to send a v2, or is it easier just for you 
> to correct them yourself?

Fixed locally (and wrapped to 72 columns), thanks!