[lib/cbuf] [app/tests] fix off by one in lib/cbuf. add a test.

Previously, if tail was == 0, and we wrote exactly enough bytes to
the end of the buffer, then head would end up at 0 as well. This
would make the buffer instaneously empty, as head == tail.

6 files changed, 142 insertions, 8 deletions

diff --git a/app/tests/cbuf_tests.c b/app/tests/cbuf_tests.c
new file mode 100644
index 00000000..9fdd05b8
--- /dev/null
+++ b/app/tests/cbuf_tests.c
@@ -0,0 +1,116 @@
+#include <assert.h>
+#include <debug.h>
+#include <err.h>
+#include <lib/cbuf.h>
+#include <lib/console.h>
+#include <lib/heap.h>
+#include <rand.h>
+#include <stdlib.h>
+
+#define ASSERT_EQ(a, b)                                            \
+    do {                                                           \
+        int _a = (a);                                              \
+        int _b = (b);                                              \
+        if (_a != _b) {                                            \
+            panic("%d != %d (%s:%d)\n", a, b, __FILE__, __LINE__); \
+        }                                                          \
+    } while (0);
+
+#define ASSERT_LEQ(a, b)                                               \
+    do {                                                               \
+        int _a = (a);                                                  \
+        int _b = (b);                                                  \
+        if (_a > _b) {                                                 \
+            panic("%d not <= %d (%s:%d)\n", a, b, __FILE__, __LINE__); \
+        }                                                              \
+    } while (0);
+
+int cbuf_tests(int argc, const cmd_args *argv)
+{
+    cbuf_t cbuf;
+
+    printf("running basic tests...\n");
+
+    cbuf_initialize(&cbuf, 16);
+
+    ASSERT_EQ(15, cbuf_space_avail(&cbuf));
+
+    ASSERT_EQ(8, cbuf_write(&cbuf, "abcdefgh", 8, false));
+
+    ASSERT_EQ(7, cbuf_space_avail(&cbuf));
+
+    // Only 7 bytes should fit since if we write all 16 bytes,
+    // head == tail and we can't distinguish it from the start case.
+    ASSERT_EQ(7, cbuf_write(&cbuf, "ijklmnop", 8, false));
+
+    ASSERT_EQ(0, cbuf_space_avail(&cbuf));
+
+    // Nothing should fit.
+    ASSERT_EQ(0, cbuf_write(&cbuf, "XXXXXXXX", 8, false));
+
+    ASSERT_EQ(0, cbuf_space_avail(&cbuf));
+
+    // Read a few bytes.
+    {
+        char buf[32];
+        ASSERT_EQ(3, cbuf_read(&cbuf, buf, 3, false));
+        for (int i = 0; i < 3; ++i) {
+            ASSERT_EQ(buf[i], 'a' + i);
+        }
+
+        // Try reading 32 bytes.
+        ASSERT_EQ(12, cbuf_read(&cbuf, buf, 32, false));
+        for (int i = 0; i < 12; ++i) {
+            ASSERT_EQ(buf[i], 'd' + i);
+        }
+    }
+
+    cbuf_reset(&cbuf);
+
+    ASSERT_EQ(15, cbuf_space_avail(&cbuf));
+
+    // Random tests. Keep writing in random chunks up to 8 bytes, then
+    // reading in chunks up to 8 bytes. Verify values.
+
+    int pos_out = 0;
+    int pos_in = 0;
+    printf("running random tests...\n");
+    while (pos_in < 256) {
+        if (pos_out < 256) {
+            // Write up to 8 bytes.
+            char buf_out[8];
+            int to_write_random = rand() & 7;
+            int to_write = MIN(to_write_random, 256 - pos_out);
+            for (int i = 0; i < to_write; ++i) {
+                buf_out[i] = pos_out + i;
+            }
+            // Advance the out pointer based on how many bytes fit.
+            int wrote = cbuf_write(&cbuf, buf_out, to_write, false);
+            ASSERT_LEQ(wrote, to_write);
+            pos_out += wrote;
+        }
+
+        // Read up to 8 bytes, make sure they are right.
+        if (pos_in < pos_out) {
+            char buf_in[8];
+            int to_read_random = rand() & 7;
+            int to_read = MIN(to_read_random, pos_out - pos_in);
+            int read = cbuf_read(&cbuf, buf_in, to_read, false);
+            ASSERT_LEQ(read, to_read);
+
+            for (int i = 0; i < read; ++i) {
+                ASSERT_EQ(pos_in + i, buf_in[i]);
+            }
+
+            pos_in += read;
+        }
+
+        ASSERT_LEQ(pos_in, pos_out);
+    }
+
+    free(cbuf.buf);
+
+    printf("cbuf tests passed\n");
+
+    return NO_ERROR;
+}
diff --git a/app/tests/include/app/tests.h b/app/tests/include/app/tests.h
index 9a8b7084..3292ac81 100644
--- a/app/tests/include/app/tests.h
+++ b/app/tests/include/app/tests.h
@@ -25,14 +25,15 @@
 
 #include <lib/console.h>
 
-int thread_tests(void);
+int cbuf_tests(int argc, const cmd_args *argv);
+int fibo(int argc, const cmd_args *argv);
 int port_tests(void);
+int spinner(int argc, const cmd_args *argv);
+int thread_tests(void);
+void benchmarks(void);
+void clock_tests(void);
 void printf_tests(void);
 void printf_tests_float(void);
-void clock_tests(void);
-void benchmarks(void);
-int fibo(int argc, const cmd_args *argv);
-int spinner(int argc, const cmd_args *argv);
 
 #endif
 
diff --git a/app/tests/rules.mk b/app/tests/rules.mk
index 545ad3b1..6ebae2c5 100644
--- a/app/tests/rules.mk
+++ b/app/tests/rules.mk
@@ -5,6 +5,7 @@ MODULE := $(LOCAL_DIR)
 MODULE_SRCS += \
     $(LOCAL_DIR)/benchmarks.c \
     $(LOCAL_DIR)/cache_tests.c \
+    $(LOCAL_DIR)/cbuf_tests.c \
     $(LOCAL_DIR)/clock_tests.c \
     $(LOCAL_DIR)/fibo.c \
     $(LOCAL_DIR)/float.c \
@@ -18,6 +19,9 @@ MODULE_SRCS += \
 
 MODULE_ARM_OVERRIDE_SRCS := \
 
+MODULE_DEPS += \
+    lib/cbuf
+
 MODULE_COMPILEFLAGS += -Wno-format -fno-builtin
 
 include make/module.mk
diff --git a/app/tests/tests.c b/app/tests/tests.c
index 779ab415..2fadd8bb 100644
--- a/app/tests/tests.c
+++ b/app/tests/tests.c
@@ -37,6 +37,7 @@ STATIC_COMMAND("clock_tests", "test clocks", (console_cmd)&clock_tests)
 STATIC_COMMAND("bench", "miscellaneous benchmarks", (console_cmd)&benchmarks)
 STATIC_COMMAND("fibo", "threaded fibonacci", (console_cmd)&fibo)
 STATIC_COMMAND("spinner", "create a spinning thread", (console_cmd)&spinner)
+STATIC_COMMAND("cbuf_tests", "test lib/cbuf", &cbuf_tests)
 STATIC_COMMAND_END(tests);
 
 #endif
diff --git a/lib/cbuf/cbuf.c b/lib/cbuf/cbuf.c
index 2195c01b..2148d633 100644
--- a/lib/cbuf/cbuf.c
+++ b/lib/cbuf/cbuf.c
@@ -84,8 +84,19 @@ size_t cbuf_write(cbuf_t *cbuf, const void *_buf, size_t len, bool canreschedule
 
     while (pos < len && cbuf_space_avail(cbuf) > 0) {
         if (cbuf->head >= cbuf->tail) {
-            write_len = MIN(valpow2(cbuf->len_pow2) - cbuf->head, len - pos);
+            if (cbuf->tail == 0) {
+                // Special case - if tail is at position 0, we can't write all
+                // the way to the end of the buffer. Otherwise, head ends up at
+                // 0, head == tail, and buffer is considered "empty" again.
+                write_len =
+                    MIN(valpow2(cbuf->len_pow2) - cbuf->head - 1, len - pos);
+            } else {
+                // Write to the end of the buffer.
+                write_len =
+                    MIN(valpow2(cbuf->len_pow2) - cbuf->head, len - pos);
+            }
         } else {
+            // Write from head to tail-1.
             write_len = MIN(cbuf->tail - cbuf->head - 1, len - pos);
         }
 
@@ -260,4 +271,3 @@ retry:
 
     return ret;
 }
-
diff --git a/lib/cbuf/include/lib/cbuf.h b/lib/cbuf/include/lib/cbuf.h
index 902628f3..d467f185 100644
--- a/lib/cbuf/include/lib/cbuf.h
+++ b/lib/cbuf/include/lib/cbuf.h
@@ -47,6 +47,7 @@ typedef struct cbuf {
  *
  * @param[in] cbuf A pointer to the cbuf structure to allocate.
  * @param[in] len The minimum number of bytes for the underlying data buffer.
+ * Must be a power of two.
  */
 void cbuf_initialize(cbuf_t *cbuf, size_t len);
 
@@ -56,7 +57,8 @@ void cbuf_initialize(cbuf_t *cbuf, size_t len);
  * Initialize a cbuf structure using the supplied buffer for internal storage.
  *
  * @param[in] cbuf A pointer to the cbuf structure to allocate.
- * @param[in] len The size of the supplied buffer, in bytes.
+ * @param[in] len The size of the supplied buffer, in bytes.  Must be a power
+ * of two.
  * @param[in] buf A pointer to the memory to be used for internal storage.
  */
 void cbuf_initialize_etc(cbuf_t *cbuf, size_t len, void *buf);