From 371a64b5148b25da8139a43afbfc3669a5059114 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 7 Jun 2026 19:54:28 -0600 Subject: [PATCH 1/9] feat: port ring_buf (sys/ring_buffer.h) from upstream Zephyr Near-verbatim port of upstream's ring_buffer.h + lib/utils/ring_buffer.c (both Apache-2.0), the canonical companion to the UART IRQ pattern and the byte transport the direct-UART shell (#31) and a modbus serial backend want. Pure arithmetic + memcpy; no kernel dependencies, nothing blocks. Byte-mode API: ring_buf_init, RING_BUF_DECLARE, reset, size/space/ capacity_get, is_empty, put/get/peek, and the zero-copy claim/finish pairs. Indices use upstream's base-offset scheme (head/tail/base of ring_buf_idx_t, byte offset = head-base with one wrap correction, RING_BUFFER_MAX_SIZE caps size to half the index range so unsigned subtraction disambiguates full vs empty). CONFIG_RING_BUFFER_LARGE widens indices to 32-bit, matching upstream. Adaptations for Boreas (documented in the files): - include layout (zephyr/sys/util.h) and lowercase upstream min() -> MIN. - toolchain shims added to sys/util.h: __ASSERT_NO_MSG, likely/unlikely, __noinit (mapped to zero-init storage -- correct here, and avoids __NOINIT_ATTR wrongly surviving deep sleep), __deprecated. Also made __ASSERT self-contained (#include for abort()). Deliberate divergence: the upstream item-mode API (ring_buf_item_put/ _get, RING_BUF_ITEM_DECLARE*) is NOT ported -- it is @deprecated upstream ("use ") and unused by the byte-transport consumers this targets. Concurrency is unchanged: lock-free for single-producer/single-consumer in separate contexts; multiple producers/consumers must serialize externally. Tests (15): accounting, round-trip, capacity saturation, partial get, NULL-discard, peek-no-consume, reset, claim/finish (put+get), surplus return, over-claim -EINVAL, and the wrap-sensitive cases -- claim splits at the physical end, put/get spanning the wrap, and a 20000-cycle stream through a 7-byte (non-power-of-two) buffer exercising base advancement and the index arithmetic across many wraps. linux suite: 221/0 x3 (206 + 15). Closes #45 Co-Authored-By: Claude Opus 4.8 (1M context) --- components/zkernel/CMakeLists.txt | 1 + components/zkernel/Kconfig | 9 + .../include/boreas/zephyr/sys/ring_buffer.h | 382 ++++++++++++++++++ .../zkernel/include/boreas/zephyr/sys/util.h | 30 ++ components/zkernel/src/ring_buf.c | 145 +++++++ test/main/CMakeLists.txt | 1 + test/main/test_main.c | 2 + test/main/test_ring_buf.c | 345 ++++++++++++++++ 8 files changed, 915 insertions(+) create mode 100644 components/zkernel/include/boreas/zephyr/sys/ring_buffer.h create mode 100644 components/zkernel/src/ring_buf.c create mode 100644 test/main/test_ring_buf.c diff --git a/components/zkernel/CMakeLists.txt b/components/zkernel/CMakeLists.txt index 6a89b51..f6355a2 100644 --- a/components/zkernel/CMakeLists.txt +++ b/components/zkernel/CMakeLists.txt @@ -5,6 +5,7 @@ set(srcs "src/k_event.c" "src/k_thread.c" "src/k_work.c" + "src/ring_buf.c" "src/sys_init.c" "src/fatal.c" ) diff --git a/components/zkernel/Kconfig b/components/zkernel/Kconfig index 5bde062..59a3e59 100644 --- a/components/zkernel/Kconfig +++ b/components/zkernel/Kconfig @@ -1,5 +1,14 @@ menu "Boreas Kernel (zkernel)" + config RING_BUFFER_LARGE + bool "Use 32-bit ring buffer indices" + default n + help + Widens the ring buffer index type from uint16_t to uint32_t, + raising the maximum ring_buf capacity from 32767 bytes to + ~2 GiB at the cost of 6 extra bytes per struct ring_buf. + Mirrors upstream Zephyr's CONFIG_RING_BUFFER_LARGE. + config ZKERNEL_MUTEX_DEBUG bool "Enable mutex lock-ordering assertions" default n diff --git a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h new file mode 100644 index 0000000..af624c3 --- /dev/null +++ b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h @@ -0,0 +1,382 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2026 Intercreate + * + * Zephyr-compatible ring buffer (sys/ring_buffer.h). Near-verbatim port + * of upstream Zephyr (include/zephyr/sys/ring_buffer.h, + * lib/utils/ring_buffer.c -- both Apache-2.0), adapted only for the + * Boreas include layout and toolchain shims (see sys/util.h). + * + * Divergence: the upstream item-mode API (ring_buf_item_put/_get, + * ring_buf_item_init, RING_BUF_ITEM_DECLARE*) is intentionally NOT + * ported -- it is @deprecated upstream ("use ") + * and the byte-mode API below is what the UART IRQ pattern, the + * direct-UART shell transport, and the modbus serial backend use. + * + * Concurrency (unchanged from upstream): the buffer is lock-free for a + * SINGLE producer and a SINGLE consumer in separate contexts (two + * threads, or thread + ISR) -- the producer touches only `put` (and + * reads `get.tail`); the consumer touches only `get` (and reads + * `put.tail`). Multiple producers OR multiple consumers must serialize + * access externally (e.g. a k_mutex or by locking interrupts). On SMP, + * pair the buffer with a k_sem to establish ordering between producer + * and consumer. + */ + +#pragma once + +#include +#include +#include + +#include "zephyr/sys/util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** @cond INTERNAL_HIDDEN */ + +#ifdef CONFIG_RING_BUFFER_LARGE +typedef uint32_t ring_buf_idx_t; +#define RING_BUFFER_MAX_SIZE (UINT32_MAX / 2) +#define RING_BUFFER_SIZE_ASSERT_MSG "Size too big" +#else +typedef uint16_t ring_buf_idx_t; +#define RING_BUFFER_MAX_SIZE (UINT16_MAX / 2) +#define RING_BUFFER_SIZE_ASSERT_MSG "Size too big, please enable CONFIG_RING_BUFFER_LARGE" +#endif + +struct ring_buf_index { + ring_buf_idx_t head; + ring_buf_idx_t tail; + ring_buf_idx_t base; +}; + +/** @endcond */ + +/** + * @brief A structure to represent a ring buffer. + */ +struct ring_buf { + /** @cond INTERNAL_HIDDEN */ + uint8_t *buffer; + struct ring_buf_index put; + struct ring_buf_index get; + uint32_t size; + /** @endcond */ +}; + +/** @cond INTERNAL_HIDDEN */ + +uint32_t ring_buf_area_claim(struct ring_buf *buf, struct ring_buf_index *ring, uint8_t **data, + uint32_t size); +int ring_buf_area_finish(struct ring_buf *buf, struct ring_buf_index *ring, uint32_t size); + +/** + * @brief Force ring_buf internal states to a given value. + * + * Any value other than 0 makes sense only in validation-testing context. + */ +static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t value) +{ + buf->put.head = buf->put.tail = buf->put.base = value; + buf->get.head = buf->get.tail = buf->get.base = value; +} + +/** @endcond */ + +/** + * @brief Define and initialize a ring buffer for byte data. + * + * This macro establishes a ring buffer of an arbitrary size. + * The basic storage unit is a byte. + * + * The ring buffer can be accessed outside the module where it is defined + * using: + * + * @code extern struct ring_buf ; @endcode + * + * @param name Name of the ring buffer. + * @param size8 Size of ring buffer (in bytes). + */ +#define RING_BUF_DECLARE(name, size8) \ + BUILD_ASSERT((size8) <= RING_BUFFER_MAX_SIZE, RING_BUFFER_SIZE_ASSERT_MSG); \ + static uint8_t __noinit _ring_buffer_data_##name[size8]; \ + struct ring_buf name = { \ + .buffer = _ring_buffer_data_##name, \ + .size = size8, \ + } + +/** + * @brief Initialize a ring buffer for byte data. + * + * This routine initializes a ring buffer, prior to its first use. It is only + * used for the byte data, for which the size is expressed in bytes. + * + * @param buf Address of ring buffer. + * @param size Ring buffer size (in bytes). + * @param data Ring buffer data area (uint8_t data[size]). + */ +static inline void ring_buf_init(struct ring_buf *buf, uint32_t size, uint8_t *data) +{ + __ASSERT(size <= RING_BUFFER_MAX_SIZE, RING_BUFFER_SIZE_ASSERT_MSG); + + buf->size = size; + buf->buffer = data; + ring_buf_internal_reset(buf, 0); +} + +/** + * @brief Determine if a ring buffer is empty. + * + * @param buf Address of ring buffer. + * + * @return true if the ring buffer is empty, or false if not. + */ +static inline bool ring_buf_is_empty(const struct ring_buf *buf) +{ + return buf->get.head == buf->put.tail; +} + +/** + * @brief Reset ring buffer state. + * + * @param buf Address of ring buffer. + */ +static inline void ring_buf_reset(struct ring_buf *buf) +{ + ring_buf_internal_reset(buf, 0); +} + +/** + * @brief Determine free space in a ring buffer. + * + * @param buf Address of ring buffer. + * + * @return Ring buffer free space (in bytes). + */ +static inline uint32_t ring_buf_space_get(const struct ring_buf *buf) +{ + ring_buf_idx_t allocated = buf->put.head - buf->get.tail; + + return buf->size - allocated; +} + +/** + * @brief Return ring buffer capacity. + * + * @param buf Address of ring buffer. + * + * @return Ring buffer capacity (in bytes). + */ +static inline uint32_t ring_buf_capacity_get(const struct ring_buf *buf) +{ + return buf->size; +} + +/** + * @brief Determine used space in a ring buffer. + * + * @param buf Address of ring buffer. + * + * @return Ring buffer space used (in bytes). + */ +static inline uint32_t ring_buf_size_get(const struct ring_buf *buf) +{ + ring_buf_idx_t available = buf->put.tail - buf->get.head; + + return available; +} + +/** + * @brief Allocate buffer for writing data to a ring buffer. + * + * With this routine, memory copying can be reduced since internal ring buffer + * can be used directly by the user. Once data is written to allocated area + * number of bytes written must be confirmed (see @ref ring_buf_put_finish). + * + * @warning + * Use cases involving multiple writers to the ring buffer must prevent + * concurrent write operations, either by preventing all writers from + * being preempted or by using a mutex to govern writes to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param data Pointer to the address. It is set to a location within + * ring buffer. + * @param size Requested allocation size (in bytes). + * + * @return Size of allocated buffer which can be smaller than requested if + * there is not enough free space or buffer wraps. + */ +static inline uint32_t ring_buf_put_claim(struct ring_buf *buf, uint8_t **data, uint32_t size) +{ + uint32_t space = ring_buf_space_get(buf); + + return ring_buf_area_claim(buf, &buf->put, data, MIN(size, space)); +} + +/** + * @brief Indicate number of bytes written to allocated buffers. + * + * The number of bytes must be equal to or lower than the sum corresponding + * to all preceding @ref ring_buf_put_claim invocations (or even 0). Surplus + * bytes will be returned to the available free buffer space. + * + * @warning + * Use cases involving multiple writers to the ring buffer must prevent + * concurrent write operations, either by preventing all writers from + * being preempted or by using a mutex to govern writes to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param size Number of valid bytes in the allocated buffers. + * + * @retval 0 Successful operation. + * @retval -EINVAL Provided @a size exceeds free space in the ring buffer. + */ +static inline int ring_buf_put_finish(struct ring_buf *buf, uint32_t size) +{ + return ring_buf_area_finish(buf, &buf->put, size); +} + +/** + * @brief Write (copy) data to a ring buffer. + * + * This routine writes data to a ring buffer @a buf. + * + * @warning + * Use cases involving multiple writers to the ring buffer must prevent + * concurrent write operations, either by preventing all writers from + * being preempted or by using a mutex to govern writes to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param data Address of data. + * @param size Data size (in bytes). + * + * @retval Number of bytes written. + */ +uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size); + +/** + * @brief Get address of a valid data in a ring buffer. + * + * With this routine, memory copying can be reduced since internal ring buffer + * can be used directly by the user. Once data is processed it must be freed + * using @ref ring_buf_get_finish. + * + * @warning + * Use cases involving multiple reads of the ring buffer must prevent + * concurrent read operations, either by preventing all readers from being + * preempted or by using a mutex to govern reads to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param data Pointer to the address. It is set to a location within + * ring buffer. + * @param size Requested size (in bytes). + * + * @return Number of valid bytes in the provided buffer which can be smaller + * than requested if there is not enough free space or buffer wraps. + */ +static inline uint32_t ring_buf_get_claim(struct ring_buf *buf, uint8_t **data, uint32_t size) +{ + uint32_t buf_size = ring_buf_size_get(buf); + + return ring_buf_area_claim(buf, &buf->get, data, MIN(size, buf_size)); +} + +/** + * @brief Indicate number of bytes read from claimed buffer. + * + * The number of bytes must be equal or lower than the sum corresponding to + * all preceding @ref ring_buf_get_claim invocations (or even 0). Surplus + * bytes will remain available in the buffer. + * + * @warning + * Use cases involving multiple reads of the ring buffer must prevent + * concurrent read operations, either by preventing all readers from being + * preempted or by using a mutex to govern reads to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param size Number of bytes that can be freed. + * + * @retval 0 Successful operation. + * @retval -EINVAL Provided @a size exceeds valid bytes in the ring buffer. + */ +static inline int ring_buf_get_finish(struct ring_buf *buf, uint32_t size) +{ + return ring_buf_area_finish(buf, &buf->get, size); +} + +/** + * @brief Read data from a ring buffer. + * + * This routine reads data from a ring buffer @a buf. + * + * @warning + * Use cases involving multiple reads of the ring buffer must prevent + * concurrent read operations, either by preventing all readers from being + * preempted or by using a mutex to govern reads to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @param buf Address of ring buffer. + * @param data Address of the output buffer. Can be NULL to discard data. + * @param size Data size (in bytes). + * + * @retval Number of bytes written to the output buffer. + */ +uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size); + +/** + * @brief Peek at data from a ring buffer. + * + * This routine reads data from a ring buffer @a buf without removing it from + * the buffer. + * + * @warning + * Use cases involving multiple reads of the ring buffer must prevent + * concurrent read operations, either by preventing all readers from being + * preempted or by using a mutex to govern reads to the ring buffer. + * + * @warning + * Ring buffer instance should not mix byte access and item access + * (calls prefixed with ring_buf_item_). + * + * @warning + * Multiple calls to peek will result in the same data being 'peeked' multiple + * times. To remove data, use either @ref ring_buf_get or @ref + * ring_buf_get_claim followed by @ref ring_buf_get_finish. + * + * @param buf Address of ring buffer. + * @param data Address of the output buffer. Cannot be NULL. + * @param size Data size (in bytes). + * + * @retval Number of bytes written to the output buffer. + */ +uint32_t ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size); + +#ifdef __cplusplus +} +#endif diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 281b9cc..5537330 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -107,6 +107,8 @@ extern "C" { * NOT safe from IRAM ISR context (ESP_LOGE + abort are flash-resident). * Use k_panic() for unrecoverable errors in ISR/IRAM context. */ #ifndef __ASSERT +#include /* abort() */ + #include "esp_log.h" #define __ASSERT(cond, msg) \ do { \ @@ -117,6 +119,34 @@ extern "C" { } while (0) #endif +/* Assertion with no message (upstream spelling). */ +#ifndef __ASSERT_NO_MSG +#define __ASSERT_NO_MSG(cond) __ASSERT((cond), "") +#endif + +/* Branch-prediction hints (upstream toolchain.h). */ +#ifndef likely +#define likely(x) __builtin_expect(!!(x), 1) +#endif +#ifndef unlikely +#define unlikely(x) __builtin_expect(!!(x), 0) +#endif + +/* Upstream places __noinit data in a section the loader does not zero, + * to save startup cost. Boreas maps it to ordinary (zero-initialized) + * storage: correct for every current user -- ring_buf, etc., set their + * own indices in the init step and never rely on uninitialized buffer + * contents -- at the cost of zeroing the buffer at boot. Mapping to + * ESP-IDF's __NOINIT_ATTR would wrongly survive deep sleep. */ +#ifndef __noinit +#define __noinit +#endif + +/* Deprecation attribute (upstream toolchain.h). */ +#ifndef __deprecated +#define __deprecated __attribute__((deprecated)) +#endif + /* IRAM-safe panic -- triggers an illegal-instruction exception caught by * the ESP-IDF panic handler (which is IRAM-resident). Produces a full * backtrace on UART and reboots. Safe to call from IRAM_ATTR ISR context. diff --git a/components/zkernel/src/ring_buf.c b/components/zkernel/src/ring_buf.c new file mode 100644 index 0000000..d4b18ba --- /dev/null +++ b/components/zkernel/src/ring_buf.c @@ -0,0 +1,145 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2026 Intercreate + * + * Zephyr-compatible ring buffer. Near-verbatim port of upstream Zephyr + * lib/utils/ring_buffer.c (Apache-2.0), adapted only for the Boreas + * include layout and toolchain shims: + * - upstream's lowercase min() (from ) -> MIN + * (sys/util.h); + * - the deprecated item-mode API (ring_buf_item_put/_get) is not + * ported -- see sys/ring_buffer.h. + * + * The index scheme is upstream's base-offset design: head/tail/base are + * ring_buf_idx_t (uint16_t, or uint32_t with CONFIG_RING_BUFFER_LARGE), + * the byte offset is (head - base) with a single wrap correction, and + * RING_BUFFER_MAX_SIZE caps size to half the index range so unsigned + * subtractions disambiguate full vs empty. Keep ring_buf_idx_t exactly + * as the header declares it -- widening only one side breaks the + * empty/full discrimination. + */ + +#include +#include + +#include "zephyr/sys/ring_buffer.h" +#include "zephyr/sys/util.h" + +uint32_t ring_buf_area_claim(struct ring_buf *buf, struct ring_buf_index *ring, uint8_t **data, + uint32_t size) +{ + ring_buf_idx_t head_offset, wrap_size; + + head_offset = ring->head - ring->base; + if (unlikely(head_offset >= buf->size)) { + /* ring->base is not yet adjusted */ + head_offset -= buf->size; + } + wrap_size = buf->size - head_offset; + size = MIN(size, wrap_size); + + *data = &buf->buffer[head_offset]; + ring->head += size; + + return size; +} + +int ring_buf_area_finish(struct ring_buf *buf, struct ring_buf_index *ring, uint32_t size) +{ + ring_buf_idx_t claimed_size, tail_offset; + + claimed_size = ring->head - ring->tail; + if (unlikely(size > claimed_size)) { + return -EINVAL; + } + + ring->tail += size; + ring->head = ring->tail; + + tail_offset = ring->tail - ring->base; + if (unlikely(tail_offset >= buf->size)) { + /* we wrapped: adjust ring->base */ + ring->base += buf->size; + } + + return 0; +} + +uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size) +{ + uint8_t *dst; + uint32_t partial_size; + uint32_t total_size = 0U; + int err; + + do { + partial_size = ring_buf_put_claim(buf, &dst, size); + if (partial_size == 0) { + break; + } + memcpy(dst, data, partial_size); + total_size += partial_size; + size -= partial_size; + data += partial_size; + } while (size != 0); + + err = ring_buf_put_finish(buf, total_size); + __ASSERT_NO_MSG(err == 0); + ARG_UNUSED(err); + + return total_size; +} + +uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size) +{ + uint8_t *src; + uint32_t partial_size; + uint32_t total_size = 0U; + int err; + + do { + partial_size = ring_buf_get_claim(buf, &src, size); + if (partial_size == 0) { + break; + } + if (data) { + memcpy(data, src, partial_size); + data += partial_size; + } + total_size += partial_size; + size -= partial_size; + } while (size != 0); + + err = ring_buf_get_finish(buf, total_size); + __ASSERT_NO_MSG(err == 0); + ARG_UNUSED(err); + + return total_size; +} + +uint32_t ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size) +{ + uint8_t *src; + uint32_t partial_size; + uint32_t total_size = 0U; + int err; + + do { + partial_size = ring_buf_get_claim(buf, &src, size); + if (partial_size == 0) { + break; + } + __ASSERT_NO_MSG(data != NULL); + memcpy(data, src, partial_size); + data += partial_size; + total_size += partial_size; + size -= partial_size; + } while (size != 0); + + /* effectively unclaim total_size bytes */ + err = ring_buf_get_finish(buf, 0); + __ASSERT_NO_MSG(err == 0); + ARG_UNUSED(err); + + return total_size; +} diff --git a/test/main/CMakeLists.txt b/test/main/CMakeLists.txt index ec93172..b04800e 100644 --- a/test/main/CMakeLists.txt +++ b/test/main/CMakeLists.txt @@ -5,6 +5,7 @@ set(test_srcs "test_dlist.c" "test_byteorder.c" "test_atomic.c" + "test_ring_buf.c" "test_k_sem.c" "test_k_mutex.c" "test_k_msgq.c" diff --git a/test/main/test_main.c b/test/main/test_main.c index 549a67e..ab5e7ab 100644 --- a/test/main/test_main.c +++ b/test/main/test_main.c @@ -15,6 +15,7 @@ void test_slist_group(void); void test_dlist_group(void); void test_byteorder_group(void); void test_atomic_group(void); +void test_ring_buf_group(void); void test_k_sem_group(void); void test_k_mutex_group(void); void test_k_msgq_group(void); @@ -53,6 +54,7 @@ void app_main(void) test_dlist_group(); test_byteorder_group(); test_atomic_group(); + test_ring_buf_group(); /* Layer 1: Kernel primitives */ test_k_sem_group(); diff --git a/test/main/test_ring_buf.c b/test/main/test_ring_buf.c new file mode 100644 index 0000000..6245c4e --- /dev/null +++ b/test/main/test_ring_buf.c @@ -0,0 +1,345 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2026 Intercreate + */ + +#include +#include + +#include "unity.h" +#include "zephyr/sys/ring_buffer.h" + +/* ---------------------------------------------------------------- + * Accounting: capacity / size / space / is_empty + * ---------------------------------------------------------------- */ + +static void test_ring_buf_init_accounting(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + + ring_buf_init(&rb, sizeof(storage), storage); + + TEST_ASSERT_EQUAL(16, ring_buf_capacity_get(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(16, ring_buf_space_get(&rb)); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); +} + +static void test_ring_buf_put_get_roundtrip(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {1, 2, 3, 4, 5}; + uint8_t out[8] = {0}; + + ring_buf_init(&rb, sizeof(storage), storage); + + TEST_ASSERT_EQUAL(5, ring_buf_put(&rb, in, sizeof(in))); + TEST_ASSERT_EQUAL(5, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(11, ring_buf_space_get(&rb)); + TEST_ASSERT_FALSE(ring_buf_is_empty(&rb)); + + TEST_ASSERT_EQUAL(5, ring_buf_get(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, out, sizeof(in)); + TEST_ASSERT_EQUAL(0, ring_buf_size_get(&rb)); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); +} + +/* ---------------------------------------------------------------- + * Full / partial semantics + * ---------------------------------------------------------------- */ + +static void test_ring_buf_put_saturates_at_capacity(void) +{ + uint8_t storage[8]; + struct ring_buf rb; + uint8_t in[12]; + + for (int i = 0; i < (int)sizeof(in); i++) { + in[i] = (uint8_t)i; + } + ring_buf_init(&rb, sizeof(storage), storage); + + /* Only capacity bytes fit; the rest is refused. */ + TEST_ASSERT_EQUAL(8, ring_buf_put(&rb, in, sizeof(in))); + TEST_ASSERT_EQUAL(0, ring_buf_space_get(&rb)); + /* A further put on a full buffer writes nothing. */ + TEST_ASSERT_EQUAL(0, ring_buf_put(&rb, in, 1)); +} + +static void test_ring_buf_get_more_than_available(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {9, 8, 7}; + uint8_t out[8] = {0}; + + ring_buf_init(&rb, sizeof(storage), storage); + ring_buf_put(&rb, in, sizeof(in)); + + /* Asking for more than present returns only what is there. */ + TEST_ASSERT_EQUAL(3, ring_buf_get(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, out, 3); +} + +static void test_ring_buf_get_null_discards(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {1, 2, 3, 4}; + + ring_buf_init(&rb, sizeof(storage), storage); + ring_buf_put(&rb, in, sizeof(in)); + + /* NULL output discards without copying. */ + TEST_ASSERT_EQUAL(4, ring_buf_get(&rb, NULL, sizeof(in))); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); +} + +/* ---------------------------------------------------------------- + * peek does not consume + * ---------------------------------------------------------------- */ + +static void test_ring_buf_peek_does_not_consume(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {5, 6, 7, 8}; + uint8_t out[4] = {0}; + + ring_buf_init(&rb, sizeof(storage), storage); + ring_buf_put(&rb, in, sizeof(in)); + + TEST_ASSERT_EQUAL(4, ring_buf_peek(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, out, sizeof(in)); + /* Still all there after peek. */ + TEST_ASSERT_EQUAL(4, ring_buf_size_get(&rb)); + + /* Second peek returns the same bytes. */ + memset(out, 0, sizeof(out)); + TEST_ASSERT_EQUAL(4, ring_buf_peek(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, out, sizeof(in)); +} + +/* ---------------------------------------------------------------- + * reset + * ---------------------------------------------------------------- */ + +static void test_ring_buf_reset_empties(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {1, 2, 3}; + + ring_buf_init(&rb, sizeof(storage), storage); + ring_buf_put(&rb, in, sizeof(in)); + TEST_ASSERT_FALSE(ring_buf_is_empty(&rb)); + + ring_buf_reset(&rb); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(16, ring_buf_space_get(&rb)); +} + +/* ---------------------------------------------------------------- + * claim / finish (zero-copy paths) + * ---------------------------------------------------------------- */ + +static void test_ring_buf_put_claim_finish(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + uint8_t *dst; + uint8_t out[8] = {0}; + + ring_buf_init(&rb, sizeof(storage), storage); + + uint32_t claimed = ring_buf_put_claim(&rb, &dst, 6); + TEST_ASSERT_EQUAL(6, claimed); + for (uint32_t i = 0; i < claimed; i++) { + dst[i] = (uint8_t)(0x10 + i); + } + TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, 6)); + TEST_ASSERT_EQUAL(6, ring_buf_size_get(&rb)); + + TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, out, 6)); + for (int i = 0; i < 6; i++) { + TEST_ASSERT_EQUAL_UINT8(0x10 + i, out[i]); + } +} + +static void test_ring_buf_put_finish_surplus_returns_space(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + uint8_t *dst; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* Claim 10 but only commit 4; the surplus 6 returns to free space. */ + TEST_ASSERT_EQUAL(10, ring_buf_put_claim(&rb, &dst, 10)); + TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, 4)); + TEST_ASSERT_EQUAL(4, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(12, ring_buf_space_get(&rb)); +} + +static void test_ring_buf_put_finish_over_claim_einval(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + uint8_t *dst; + + ring_buf_init(&rb, sizeof(storage), storage); + + TEST_ASSERT_EQUAL(4, ring_buf_put_claim(&rb, &dst, 4)); + /* Committing more than claimed is rejected. */ + TEST_ASSERT_EQUAL(-EINVAL, ring_buf_put_finish(&rb, 5)); +} + +static void test_ring_buf_get_claim_finish(void) +{ + uint8_t storage[16]; + struct ring_buf rb; + const uint8_t in[] = {1, 2, 3, 4, 5, 6}; + uint8_t *src; + + ring_buf_init(&rb, sizeof(storage), storage); + ring_buf_put(&rb, in, sizeof(in)); + + uint32_t got = ring_buf_get_claim(&rb, &src, 6); + TEST_ASSERT_EQUAL(6, got); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, src, 6); + + /* Free only 2; the other 4 remain available. */ + TEST_ASSERT_EQUAL(0, ring_buf_get_finish(&rb, 2)); + TEST_ASSERT_EQUAL(4, ring_buf_size_get(&rb)); +} + +/* ---------------------------------------------------------------- + * Wraparound: a claim never crosses the physical end of the buffer, + * so a put/get spanning the wrap is split across two claims. This is + * the load-bearing index arithmetic (base-offset scheme). + * ---------------------------------------------------------------- */ + +static void test_ring_buf_put_claim_wraps(void) +{ + uint8_t storage[8]; + struct ring_buf rb; + uint8_t *dst; + const uint8_t a[] = {1, 2, 3, 4, 5, 6}; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* Advance head/tail to offset 6, then drain so space==capacity + * but the write offset sits near the end. */ + ring_buf_put(&rb, a, sizeof(a)); + TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, NULL, sizeof(a))); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); + + /* Now a claim for 8 returns only the 2 contiguous bytes to the + * physical end; the rest needs a second claim after it. */ + uint32_t first = ring_buf_put_claim(&rb, &dst, 8); + TEST_ASSERT_EQUAL(2, first); + dst[0] = 0xAA; + dst[1] = 0xBB; + + uint32_t second = ring_buf_put_claim(&rb, &dst, 8); + TEST_ASSERT_EQUAL(6, second); /* wrapped to the start */ + for (uint32_t i = 0; i < second; i++) { + dst[i] = (uint8_t)(0xC0 + i); + } + TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, first + second)); + TEST_ASSERT_EQUAL(8, ring_buf_size_get(&rb)); +} + +/* ring_buf_put/get themselves loop over the wrap, so a single call that + * spans the boundary must still round-trip the bytes in order. */ +static void test_ring_buf_put_get_across_wrap(void) +{ + uint8_t storage[8]; + struct ring_buf rb; + uint8_t out[8] = {0}; + const uint8_t a[] = {10, 11, 12, 13, 14}; + const uint8_t b[] = {20, 21, 22, 23, 24, 25}; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* Push the offset forward by 5, drain it. */ + ring_buf_put(&rb, a, sizeof(a)); + ring_buf_get(&rb, out, sizeof(a)); + + /* This 6-byte write starts at offset 5 and wraps the 8-byte buffer. */ + TEST_ASSERT_EQUAL(6, ring_buf_put(&rb, b, sizeof(b))); + memset(out, 0, sizeof(out)); + TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, out, sizeof(b))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(b, out, sizeof(b)); +} + +/* Many wrap cycles exercise base advancement and the unsigned index + * subtractions across the full ring_buf_idx_t range without drift. */ +static void test_ring_buf_many_wrap_cycles(void) +{ + uint8_t storage[7]; /* non-power-of-two on purpose */ + struct ring_buf rb; + uint8_t next_write = 0; + uint8_t next_read = 0; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* Far more bytes than fit in ring_buf_idx_t at a time, streamed in + * 5-byte chunks through a 7-byte buffer so the offset wraps often. */ + for (int cycle = 0; cycle < 20000; cycle++) { + uint8_t chunk[5]; + + for (int i = 0; i < 5; i++) { + chunk[i] = next_write++; + } + TEST_ASSERT_EQUAL(5, ring_buf_put(&rb, chunk, sizeof(chunk))); + + uint8_t out[5] = {0}; + TEST_ASSERT_EQUAL(5, ring_buf_get(&rb, out, sizeof(out))); + for (int i = 0; i < 5; i++) { + TEST_ASSERT_EQUAL_UINT8(next_read++, out[i]); + } + } + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); +} + +/* ---------------------------------------------------------------- + * RING_BUF_DECLARE static instance + * ---------------------------------------------------------------- */ + +RING_BUF_DECLARE(declared_rb, 12); + +static void test_ring_buf_declare_usable(void) +{ + const uint8_t in[] = {0x55, 0x66, 0x77}; + uint8_t out[3] = {0}; + + TEST_ASSERT_EQUAL(12, ring_buf_capacity_get(&declared_rb)); + TEST_ASSERT_TRUE(ring_buf_is_empty(&declared_rb)); + + TEST_ASSERT_EQUAL(3, ring_buf_put(&declared_rb, in, sizeof(in))); + TEST_ASSERT_EQUAL(3, ring_buf_get(&declared_rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(in, out, sizeof(in)); +} + +void test_ring_buf_group(void) +{ + RUN_TEST(test_ring_buf_init_accounting); + RUN_TEST(test_ring_buf_put_get_roundtrip); + RUN_TEST(test_ring_buf_put_saturates_at_capacity); + RUN_TEST(test_ring_buf_get_more_than_available); + RUN_TEST(test_ring_buf_get_null_discards); + RUN_TEST(test_ring_buf_peek_does_not_consume); + RUN_TEST(test_ring_buf_reset_empties); + RUN_TEST(test_ring_buf_put_claim_finish); + RUN_TEST(test_ring_buf_put_finish_surplus_returns_space); + RUN_TEST(test_ring_buf_put_finish_over_claim_einval); + RUN_TEST(test_ring_buf_get_claim_finish); + RUN_TEST(test_ring_buf_put_claim_wraps); + RUN_TEST(test_ring_buf_put_get_across_wrap); + RUN_TEST(test_ring_buf_many_wrap_cycles); + RUN_TEST(test_ring_buf_declare_usable); +} From 960eab38d338a3d844f55f2b7ef2853ef83fe0d6 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 7 Jun 2026 20:01:02 -0600 Subject: [PATCH 2/9] fix: ring_buf review feedback -- restore RING_BUF_INIT, attribution, tests Review fan-out (fidelity + conformance + adversarial) findings folded: - Restore the public RING_BUF_INIT macro (upstream API completeness; RING_BUF_DECLARE is now defined in terms of it, matching upstream structure). A downstream `struct ring_buf rb = RING_BUF_INIT(...)` now compiles. - Retain upstream's "Copyright (c) 2015 Intel Corporation" line on the two verbatim files alongside the Intercreate port line (Apache-2.0 4(c) preservation for a substantial verbatim derivative; the repo is going public). - Document the __noinit limitation: Boreas wires no .noinit section, so no-init/retained-RAM semantics are not available (storage is plain zero-init BSS). - Tests: assert ring_buf_put_claim returns 0 on a full buffer (the zero-copy path UART IRQ uses), and assert ring_buf_space_get across a wrap (the put.head - get.tail subtraction path). Adversarial index-arithmetic probe found zero constructible failures: the UINT16_MAX/2 cap is exact and inclusive (size=32768 would alias head-base to 0 mod 65536 and is rejected at build/init), and the non-power-of-two-size x uint16-index-wrap case is safe because only modular differences are used and tail-base is held in [0,size) by one base bump per wrap. The 20000-cycle test crosses the uint16 boundary, so it covers that class. linux suite: 221/0 x3. Refs #45 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../include/boreas/zephyr/sys/ring_buffer.h | 23 +++++++++++++++---- .../zkernel/include/boreas/zephyr/sys/util.h | 7 +++++- components/zkernel/src/ring_buf.c | 3 ++- test/main/test_ring_buf.c | 13 +++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h index af624c3..85d882d 100644 --- a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h +++ b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h @@ -1,6 +1,7 @@ /* * SPDX-License-Identifier: Apache-2.0 - * Copyright 2026 Intercreate + * Copyright (c) 2015 Intel Corporation (upstream Zephyr original) + * Copyright 2026 Intercreate (Boreas port) * * Zephyr-compatible ring buffer (sys/ring_buffer.h). Near-verbatim port * of upstream Zephyr (include/zephyr/sys/ring_buffer.h, @@ -86,6 +87,21 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t /** @endcond */ +/** + * @brief Statically initialize a ring buffer for byte data. + * + * For use in a struct ring_buf initializer with a caller-provided data + * area, e.g. `struct ring_buf rb = RING_BUF_INIT(buf, sizeof(buf));`. + * + * @param buf Pointer to the data area (uint8_t[]). + * @param size8 Size of the data area (in bytes). + */ +#define RING_BUF_INIT(buf, size8) \ + { \ + .buffer = (buf), \ + .size = (size8), \ + } + /** * @brief Define and initialize a ring buffer for byte data. * @@ -103,10 +119,7 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t #define RING_BUF_DECLARE(name, size8) \ BUILD_ASSERT((size8) <= RING_BUFFER_MAX_SIZE, RING_BUFFER_SIZE_ASSERT_MSG); \ static uint8_t __noinit _ring_buffer_data_##name[size8]; \ - struct ring_buf name = { \ - .buffer = _ring_buffer_data_##name, \ - .size = size8, \ - } + struct ring_buf name = RING_BUF_INIT(_ring_buffer_data_##name, size8) /** * @brief Initialize a ring buffer for byte data. diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 5537330..9447cb4 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -137,7 +137,12 @@ extern "C" { * storage: correct for every current user -- ring_buf, etc., set their * own indices in the init step and never rely on uninitialized buffer * contents -- at the cost of zeroing the buffer at boot. Mapping to - * ESP-IDF's __NOINIT_ATTR would wrongly survive deep sleep. */ + * ESP-IDF's __NOINIT_ATTR would wrongly survive deep sleep. + * + * Known limitation: Boreas wires up no `.noinit` linker section, so a + * user who genuinely needs no-init/retained-RAM semantics (e.g. data + * preserved across a warm reset) does NOT get them here -- the storage + * is plain zero-initialized BSS. */ #ifndef __noinit #define __noinit #endif diff --git a/components/zkernel/src/ring_buf.c b/components/zkernel/src/ring_buf.c index d4b18ba..fde9006 100644 --- a/components/zkernel/src/ring_buf.c +++ b/components/zkernel/src/ring_buf.c @@ -1,6 +1,7 @@ /* * SPDX-License-Identifier: Apache-2.0 - * Copyright 2026 Intercreate + * Copyright (c) 2015 Intel Corporation (upstream Zephyr original) + * Copyright 2026 Intercreate (Boreas port) * * Zephyr-compatible ring buffer. Near-verbatim port of upstream Zephyr * lib/utils/ring_buffer.c (Apache-2.0), adapted only for the Boreas diff --git a/test/main/test_ring_buf.c b/test/main/test_ring_buf.c index 6245c4e..1f4ebd6 100644 --- a/test/main/test_ring_buf.c +++ b/test/main/test_ring_buf.c @@ -66,6 +66,11 @@ static void test_ring_buf_put_saturates_at_capacity(void) TEST_ASSERT_EQUAL(0, ring_buf_space_get(&rb)); /* A further put on a full buffer writes nothing. */ TEST_ASSERT_EQUAL(0, ring_buf_put(&rb, in, 1)); + + /* The zero-copy claim path (what the UART IRQ uses) must also + * report no room: put_claim returns 0 on a full buffer. */ + uint8_t *dst; + TEST_ASSERT_EQUAL(0, ring_buf_put_claim(&rb, &dst, 1)); } static void test_ring_buf_get_more_than_available(void) @@ -271,9 +276,17 @@ static void test_ring_buf_put_get_across_wrap(void) /* This 6-byte write starts at offset 5 and wraps the 8-byte buffer. */ TEST_ASSERT_EQUAL(6, ring_buf_put(&rb, b, sizeof(b))); + /* Accounting must be correct across the wrap: space_get uses the + * other unsigned-subtraction path (put.head - get.tail). */ + TEST_ASSERT_EQUAL(6, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(2, ring_buf_space_get(&rb)); + memset(out, 0, sizeof(out)); TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, out, sizeof(b))); TEST_ASSERT_EQUAL_UINT8_ARRAY(b, out, sizeof(b)); + /* Fully drained after the wrap. */ + TEST_ASSERT_EQUAL(0, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(8, ring_buf_space_get(&rb)); } /* Many wrap cycles exercise base advancement and the unsigned index From bfba13a587a9b12c417f03b49f06302b1326f77b Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 7 Jun 2026 21:19:58 -0600 Subject: [PATCH 3/9] docs: retain upstream copyright on the verbatim-derived bits (pre-public) A pre-public licensing audit compared every Zephyr-API file against the upstream original (function bodies and macro expansions, not just names). Apache-2.0 4(c) requires retaining the upstream copyright only where actual copyrightable expression was copied. Retain upstream copyright (genuine verbatim/near-verbatim content): - sys/util.h: IS_ENABLED's _XXXX##/_YYYY token-paste trick is upstream verbatim -> Copyright (c) 2011-2014 Wind River Systems, Inc. - sys/byteorder.h: the 32-bit be/le composition mirrors upstream's canonical chaining -> Copyright (c) 2015-2016 Intel Corporation. - (sys/ring_buffer.h + ring_buf.c already carry Copyright (c) 2015 Intel Corporation from the port commit.) Deliberately NOT attributed (independent reimplementations -- adding an upstream notice would be false attribution): sys/dlist.h, sys/slist.h (upstream is macro-generated; ours is hand-written over a different struct layout), sys/atomic.h (inline __atomic wrappers vs upstream's extern decls), sys/time_units.h (us-stored k_timeout_t + FreeRTOS ticks vs upstream's Hz-based z_tmcvt), and all of zshell/zsys/zdevice (the shell, logging, and device model are independent implementations over ESP-IDF/FreeRTOS; only the public API surface matches Zephyr). Co-Authored-By: Claude Opus 4.8 (1M context) --- components/zkernel/include/boreas/zephyr/sys/byteorder.h | 8 ++++++-- components/zkernel/include/boreas/zephyr/sys/util.h | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/sys/byteorder.h b/components/zkernel/include/boreas/zephyr/sys/byteorder.h index 87f589d..474dbae 100644 --- a/components/zkernel/include/boreas/zephyr/sys/byteorder.h +++ b/components/zkernel/include/boreas/zephyr/sys/byteorder.h @@ -1,9 +1,13 @@ /* * SPDX-License-Identifier: Apache-2.0 - * Copyright 2026 Intercreate + * Copyright (c) 2015-2016 Intel Corporation (upstream Zephyr) + * Copyright 2026 Intercreate (Boreas) * * Zephyr-compatible byte-order helpers for little- and big-endian - * 16- and 32-bit access over byte buffers. + * 16- and 32-bit access over byte buffers. The big/little-endian + * 32-bit composition mirrors upstream's canonical chaining closely + * enough to retain the upstream copyright; the 16-bit leaf functions + * are independently written. * * @note Upstream Zephyr also provides 24/48/64-bit variants * (sys_{get,put}_{le,be}{24,48,64}). These are intentionally diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 9447cb4..117513c 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -1,8 +1,12 @@ /* * SPDX-License-Identifier: Apache-2.0 - * Copyright 2026 Intercreate + * Copyright (c) 2011-2014 Wind River Systems, Inc. (upstream Zephyr) + * Copyright 2026 Intercreate (Boreas) * - * Zephyr-compatible utility macros. + * Zephyr-compatible utility macros. Most macros here are universal C + * idioms (independently written), but the IS_ENABLED machinery (the + * _XXXX##/_YYYY token-paste trick) is taken verbatim from upstream + * Zephyr, hence the retained upstream copyright. */ #pragma once From 74af6dc7db1e3c35973152b93daa6cf7df9deeac Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Sun, 7 Jun 2026 21:49:07 -0600 Subject: [PATCH 4/9] fix: ring_buf /code-review feedback -- explicit sdkconfig.h, docs, test Folded findings from the code-review fan-out (no correctness bugs were found; these are robustness/coverage/doc improvements): - ring_buffer.h + ring_buf.c now #include "sdkconfig.h" explicitly. struct ring_buf's index width is selected by CONFIG_RING_BUFFER_LARGE, so the config must be visible wherever the struct is defined. It was safe before only by a transitive chain (util.h -> esp_attr.h -> sdkconfig.h); making it explicit removes a silent cross-TU ABI hazard if that chain ever changes, and matches the sibling .c convention. - util.h: corrected the __noinit comment. The prior rationale was wrong (ESP-IDF DOES ship __NOINIT_ATTR + a .noinit section, and that attribute survives warm reset, not deep sleep). The real reasons to map __noinit to plain BSS: ring_buf never reads unwritten bytes (so no-init vs zero-init is invisible), and plain BSS avoids a section attribute that does not port to the Mach-O host toolchain. The no-init limitation is documented. - util.h: dropped the unused __deprecated shim (no users; add toolchain shims when a real user lands). likely/unlikely kept (unlikely is used by ring_buf.c; both are #ifndef-guarded and identical in value to ESP-IDF's, so the include-order race is codegen-only). - test: added test_ring_buf_full_empty_across_wrap -- fills to full / drains to empty for 40 cycles (163 KB through a 4 KB buffer, crossing the uint16 index wrap ~2.5x), asserting the full (space==0, put refused) and empty discriminations AT high `allocated` across the wrap. The existing many-wrap test drains within a 7-byte span, so it never exercised full-vs-empty near the index wrap. linux suite: 222/0 x3. Refs #45 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../include/boreas/zephyr/sys/ring_buffer.h | 6 ++ .../zkernel/include/boreas/zephyr/sys/util.h | 25 ++++---- components/zkernel/src/ring_buf.c | 2 + test/main/test_ring_buf.c | 60 +++++++++++++++++++ 4 files changed, 79 insertions(+), 14 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h index 85d882d..7bdb6dc 100644 --- a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h +++ b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h @@ -30,6 +30,12 @@ #include #include +/* struct ring_buf's index width is selected by CONFIG_RING_BUFFER_LARGE + * below, so the config must be visible wherever this struct is defined + * -- include it explicitly rather than relying on a transitive chain + * (the library and every consumer must agree on the layout). */ +#include "sdkconfig.h" + #include "zephyr/sys/util.h" #ifdef __cplusplus diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 117513c..0c9bb20 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -137,25 +137,22 @@ extern "C" { #endif /* Upstream places __noinit data in a section the loader does not zero, - * to save startup cost. Boreas maps it to ordinary (zero-initialized) - * storage: correct for every current user -- ring_buf, etc., set their - * own indices in the init step and never rely on uninitialized buffer - * contents -- at the cost of zeroing the buffer at boot. Mapping to - * ESP-IDF's __NOINIT_ATTR would wrongly survive deep sleep. + * to save startup cost. Boreas maps it to ordinary zero-initialized BSS + * rather than ESP-IDF's __NOINIT_ATTR (which is a `.noinit` section + * attribute): for the only user, ring_buf, a RING_BUF_DECLARE'd buffer + * has its indices zero-initialized by the struct's static initializer + * and never reads buffer bytes it has not written, so no-init vs + * zero-init is behaviorally invisible. Plain BSS also keeps the shim + * free of a section attribute (which does not port cleanly to the + * Mach-O host toolchain used for the linux test target). * - * Known limitation: Boreas wires up no `.noinit` linker section, so a - * user who genuinely needs no-init/retained-RAM semantics (e.g. data - * preserved across a warm reset) does NOT get them here -- the storage - * is plain zero-initialized BSS. */ + * Known limitation: genuine no-init semantics are NOT provided -- code + * that needs data left uninitialized (or retained across a warm reset) + * must use ESP-IDF's __NOINIT_ATTR / RTC_NOINIT_ATTR directly. */ #ifndef __noinit #define __noinit #endif -/* Deprecation attribute (upstream toolchain.h). */ -#ifndef __deprecated -#define __deprecated __attribute__((deprecated)) -#endif - /* IRAM-safe panic -- triggers an illegal-instruction exception caught by * the ESP-IDF panic handler (which is IRAM-resident). Produces a full * backtrace on UART and reboots. Safe to call from IRAM_ATTR ISR context. diff --git a/components/zkernel/src/ring_buf.c b/components/zkernel/src/ring_buf.c index fde9006..ca1a873 100644 --- a/components/zkernel/src/ring_buf.c +++ b/components/zkernel/src/ring_buf.c @@ -23,6 +23,8 @@ #include #include +#include "sdkconfig.h" /* CONFIG_RING_BUFFER_LARGE selects the index width */ + #include "zephyr/sys/ring_buffer.h" #include "zephyr/sys/util.h" diff --git a/test/main/test_ring_buf.c b/test/main/test_ring_buf.c index 1f4ebd6..574de7f 100644 --- a/test/main/test_ring_buf.c +++ b/test/main/test_ring_buf.c @@ -319,6 +319,65 @@ static void test_ring_buf_many_wrap_cycles(void) TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); } +/* Fill-to-full / drain-to-empty repeatedly, enough cycles to carry the + * uint16_t indices across their 65536 wrap, asserting the full and + * empty discriminations AT high `allocated` each cycle. The + * many-wrap-cycles test above drains within a 7-byte span so allocated + * never approaches capacity; this one holds the buffer completely full + * across the index wrap, exercising space_get == 0 / put returning 0 + * (full) vs is_empty (empty) right where the modular subtraction must + * stay unambiguous. */ +static void test_ring_buf_full_empty_across_wrap(void) +{ + static uint8_t storage[4096]; + struct ring_buf rb; + uint8_t *dst; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* 4096 bytes/cycle; 40 cycles = 163840 bytes, crossing the 65536 + * index wrap ~2.5 times. */ + for (int cycle = 0; cycle < 40; cycle++) { + uint8_t seed = (uint8_t)cycle; + + /* Fill to exactly full via the claim path. */ + uint32_t total = 0; + while (total < sizeof(storage)) { + uint32_t n = ring_buf_put_claim(&rb, &dst, sizeof(storage) - total); + TEST_ASSERT_NOT_EQUAL(0, n); + for (uint32_t i = 0; i < n; i++) { + dst[i] = (uint8_t)(seed + total + i); + } + total += n; + TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, n)); + } + + /* Full: no space, both put paths refuse. */ + TEST_ASSERT_EQUAL(sizeof(storage), ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_space_get(&rb)); + TEST_ASSERT_FALSE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_put_claim(&rb, &dst, 1)); + + /* Drain fully, verifying the bytes survived the wrap. */ + uint32_t got = 0; + while (got < sizeof(storage)) { + uint8_t *src; + uint32_t n = ring_buf_get_claim(&rb, &src, sizeof(storage) - got); + TEST_ASSERT_NOT_EQUAL(0, n); + for (uint32_t i = 0; i < n; i++) { + TEST_ASSERT_EQUAL_UINT8((uint8_t)(seed + got + i), src[i]); + } + got += n; + TEST_ASSERT_EQUAL(0, ring_buf_get_finish(&rb, n)); + } + + /* Empty: discrimination from the full state above. */ + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_size_get(&rb)); + TEST_ASSERT_EQUAL(sizeof(storage), ring_buf_space_get(&rb)); + } +} + /* ---------------------------------------------------------------- * RING_BUF_DECLARE static instance * ---------------------------------------------------------------- */ @@ -354,5 +413,6 @@ void test_ring_buf_group(void) RUN_TEST(test_ring_buf_put_claim_wraps); RUN_TEST(test_ring_buf_put_get_across_wrap); RUN_TEST(test_ring_buf_many_wrap_cycles); + RUN_TEST(test_ring_buf_full_empty_across_wrap); RUN_TEST(test_ring_buf_declare_usable); } From 7444a6c144c038e816db7a30cb73a070eaa13867 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Wed, 10 Jun 2026 13:43:29 -0600 Subject: [PATCH 5/9] refactor: split toolchain.h and sys/__assert.h out of util.h util.h was accreting symbols whose upstream homes are (likely/unlikely, __weak, ALWAYS_INLINE, __noinit) and (__ASSERT, __ASSERT_NO_MSG), while the repo otherwise mirrors upstream include paths exactly -- the devlog TODO already records a downstream smf port hand-editing upstream #includes because of this. Thin headers at the upstream paths fix that now, while they have 1-2 consumers; util.h re-includes both so existing ports keep working. Also two correctness fixes folded in: - __noinit now maps to __NOINIT_ATTR (real .noinit on hardware, no-op on the linux target via esp_attr.h's CONFIG_IDF_TARGET_LINUX gate in both IDF v5.4 and v5.5). The old empty shim silently gave zero-init BSS semantics to any future warm-reset-retention user, and its stated Mach-O blocker was false -- the macOS host build of the linux target compiles clean. - likely/unlikely now document the divergence from esp_compiler.h (ESP-IDF's are CONFIG_COMPILER_OPTIMIZATION_PERF-gated; Boreas matches upstream Zephyr's unconditional __builtin_expect; both are #ifndef-guarded so the include-order race is codegen-only). Co-Authored-By: Claude Fable 5 --- .../include/boreas/zephyr/sys/__assert.h | 33 ++++++++++ .../zkernel/include/boreas/zephyr/sys/util.h | 62 ++----------------- .../zkernel/include/boreas/zephyr/toolchain.h | 50 +++++++++++++++ 3 files changed, 89 insertions(+), 56 deletions(-) create mode 100644 components/zkernel/include/boreas/zephyr/sys/__assert.h create mode 100644 components/zkernel/include/boreas/zephyr/toolchain.h diff --git a/components/zkernel/include/boreas/zephyr/sys/__assert.h b/components/zkernel/include/boreas/zephyr/sys/__assert.h new file mode 100644 index 0000000..bade260 --- /dev/null +++ b/components/zkernel/include/boreas/zephyr/sys/__assert.h @@ -0,0 +1,33 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2026 Intercreate + * + * Zephyr-compatible runtime assertions (upstream + * spellings). + * + * Divergence: upstream compiles these out unless CONFIG_ASSERT=y + * (default n); Boreas keeps them always on -- they log and abort. + * NOT safe from IRAM ISR context (ESP_LOGE + abort are flash-resident). + * Use k_panic() (sys/util.h) for unrecoverable errors in ISR/IRAM + * context. + */ + +#pragma once + +#ifndef __ASSERT +#include /* abort() */ + +#include "esp_log.h" +#define __ASSERT(cond, msg) \ + do { \ + if (!(cond)) { \ + ESP_LOGE("ASSERT", "%s at %s:%d", (msg), __FILE__, __LINE__); \ + abort(); \ + } \ + } while (0) +#endif + +/* Assertion with no message (upstream spelling). */ +#ifndef __ASSERT_NO_MSG +#define __ASSERT_NO_MSG(cond) __ASSERT((cond), "") +#endif diff --git a/components/zkernel/include/boreas/zephyr/sys/util.h b/components/zkernel/include/boreas/zephyr/sys/util.h index 0c9bb20..9229298 100644 --- a/components/zkernel/include/boreas/zephyr/sys/util.h +++ b/components/zkernel/include/boreas/zephyr/sys/util.h @@ -13,6 +13,12 @@ #include "esp_attr.h" +/* Compatibility re-exports: upstream code reaches these symbols through + * and , but historical + * Boreas ports include only this header -- keep both paths working. */ +#include "zephyr/sys/__assert.h" +#include "zephyr/toolchain.h" + #ifdef __cplusplus extern "C" { #endif @@ -97,62 +103,6 @@ extern "C" { #define ALIGNED(x) __attribute__((__aligned__(x))) #endif -/* Weak symbol */ -#ifndef __weak -#define __weak __attribute__((__weak__)) -#endif - -/* Always inline — undef any prior definition to guarantee `inline` is present - * (RISC-V GCC errors without it under -Werror=attributes). */ -#undef ALWAYS_INLINE -#define ALWAYS_INLINE __attribute__((always_inline)) inline - -/* Runtime assertion -- logs and aborts. - * NOT safe from IRAM ISR context (ESP_LOGE + abort are flash-resident). - * Use k_panic() for unrecoverable errors in ISR/IRAM context. */ -#ifndef __ASSERT -#include /* abort() */ - -#include "esp_log.h" -#define __ASSERT(cond, msg) \ - do { \ - if (!(cond)) { \ - ESP_LOGE("ASSERT", "%s at %s:%d", (msg), __FILE__, __LINE__); \ - abort(); \ - } \ - } while (0) -#endif - -/* Assertion with no message (upstream spelling). */ -#ifndef __ASSERT_NO_MSG -#define __ASSERT_NO_MSG(cond) __ASSERT((cond), "") -#endif - -/* Branch-prediction hints (upstream toolchain.h). */ -#ifndef likely -#define likely(x) __builtin_expect(!!(x), 1) -#endif -#ifndef unlikely -#define unlikely(x) __builtin_expect(!!(x), 0) -#endif - -/* Upstream places __noinit data in a section the loader does not zero, - * to save startup cost. Boreas maps it to ordinary zero-initialized BSS - * rather than ESP-IDF's __NOINIT_ATTR (which is a `.noinit` section - * attribute): for the only user, ring_buf, a RING_BUF_DECLARE'd buffer - * has its indices zero-initialized by the struct's static initializer - * and never reads buffer bytes it has not written, so no-init vs - * zero-init is behaviorally invisible. Plain BSS also keeps the shim - * free of a section attribute (which does not port cleanly to the - * Mach-O host toolchain used for the linux test target). - * - * Known limitation: genuine no-init semantics are NOT provided -- code - * that needs data left uninitialized (or retained across a warm reset) - * must use ESP-IDF's __NOINIT_ATTR / RTC_NOINIT_ATTR directly. */ -#ifndef __noinit -#define __noinit -#endif - /* IRAM-safe panic -- triggers an illegal-instruction exception caught by * the ESP-IDF panic handler (which is IRAM-resident). Produces a full * backtrace on UART and reboots. Safe to call from IRAM_ATTR ISR context. diff --git a/components/zkernel/include/boreas/zephyr/toolchain.h b/components/zkernel/include/boreas/zephyr/toolchain.h new file mode 100644 index 0000000..07f085c --- /dev/null +++ b/components/zkernel/include/boreas/zephyr/toolchain.h @@ -0,0 +1,50 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright 2026 Intercreate + * + * Zephyr-compatible toolchain shims (upstream + * spellings), kept thin so near-verbatim ports can keep their upstream + * #include shape. All definitions are #ifndef-guarded: a TU that + * already got a definition elsewhere keeps the first one it saw. + */ + +#pragma once + +#include "esp_attr.h" + +/* Branch-prediction hints. + * Divergence from ESP-IDF: esp_compiler.h defines likely/unlikely as + * plain (x) unless CONFIG_COMPILER_OPTIMIZATION_PERF is set; Boreas + * matches upstream Zephyr's unconditional __builtin_expect instead. + * Both definitions are #ifndef-guarded, so include order picks the + * winner per TU -- the difference is codegen-only (identical + * semantics either way). */ +#ifndef likely +#define likely(x) __builtin_expect(!!(x), 1) +#endif +#ifndef unlikely +#define unlikely(x) __builtin_expect(!!(x), 0) +#endif + +/* Weak symbol */ +#ifndef __weak +#define __weak __attribute__((__weak__)) +#endif + +/* Always inline -- undef any prior definition to guarantee `inline` is + * present (RISC-V GCC errors without it under -Werror=attributes). */ +#undef ALWAYS_INLINE +#define ALWAYS_INLINE __attribute__((always_inline)) inline + +/* Upstream places __noinit data in a section the loader does not zero + * (so it also survives a warm reset). Mapped to ESP-IDF's + * __NOINIT_ATTR: a genuine .noinit section on hardware targets, and a + * no-op on the linux/host test target (esp_attr.h's _SECTION_ATTR_IMPL + * expands to nothing under CONFIG_IDF_TARGET_LINUX), where __noinit + * data is ordinary zero-initialized BSS instead. Code that must retain + * data across a warm reset therefore cannot rely on this shim on the + * host target; on hardware, prefer RTC_NOINIT_ATTR when the data must + * also survive deep sleep. */ +#ifndef __noinit +#define __noinit __NOINIT_ATTR +#endif From 739fa2700b74e9605027cf701b0c7825210df131 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Wed, 10 Jun 2026 13:43:45 -0600 Subject: [PATCH 6/9] fix: ring_buf ISR safety -- K_ISR_SAFE + IRAM-safe k_panic checks The header advertises the byte API for thread + ISR use (the UART IRQ pattern), and the repo convention is that ISR-callable primitives are K_ISR_SAFE (k_sem, k_msgq, k_event, k_work): ESP-IDF ISRs registered with ESP_INTR_FLAG_IRAM fire during flash-cache-disabled windows (e.g. NVS commits), where calling flash-resident code panics with "Cache disabled but cached memory region accessed". All five functions are now IRAM-resident (verified via nm: 0x4037xxxx alongside k_sem_give). The internal __ASSERT_NO_MSGs are replaced with k_panic() checks: the upstream asserts compile out under its default CONFIG_ASSERT=n, but Boreas's __ASSERT is always on and its ESP_LOGE+abort failure path is itself flash-resident (util.h documents it as not IRAM-safe and prescribes k_panic() -- k_work.c already set the precedent of replacing upstream asserts on K_ISR_SAFE paths). memcpy is ROM-resident on ESP32-S3, so the copy loops remain safe. ring_buf_peek's NULL check is also hoisted above the copy loop so the documented "Cannot be NULL" contract is enforced on an empty buffer too, instead of silently returning 0 until data arrives. Co-Authored-By: Claude Fable 5 --- components/zkernel/src/ring_buf.c | 48 ++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/components/zkernel/src/ring_buf.c b/components/zkernel/src/ring_buf.c index ca1a873..6bd5fea 100644 --- a/components/zkernel/src/ring_buf.c +++ b/components/zkernel/src/ring_buf.c @@ -9,7 +9,14 @@ * - upstream's lowercase min() (from ) -> MIN * (sys/util.h); * - the deprecated item-mode API (ring_buf_item_put/_get) is not - * ported -- see sys/ring_buffer.h. + * ported -- see sys/ring_buffer.h; + * - all functions are K_ISR_SAFE (IRAM-resident) and the internal + * __ASSERT_NO_MSGs are replaced with IRAM-safe k_panic() checks: + * the byte API is documented for thread + ISR use, and ESP-IDF + * ISRs registered with ESP_INTR_FLAG_IRAM fire during + * flash-cache-disabled windows where flash-resident code (and + * __ASSERT's ESP_LOGE + abort path) would fault. memcpy is + * ROM-resident on ESP32-S3, so the copy loops remain safe. * * The index scheme is upstream's base-offset design: head/tail/base are * ring_buf_idx_t (uint16_t, or uint32_t with CONFIG_RING_BUFFER_LARGE), @@ -28,8 +35,8 @@ #include "zephyr/sys/ring_buffer.h" #include "zephyr/sys/util.h" -uint32_t ring_buf_area_claim(struct ring_buf *buf, struct ring_buf_index *ring, uint8_t **data, - uint32_t size) +uint32_t K_ISR_SAFE ring_buf_area_claim(struct ring_buf *buf, struct ring_buf_index *ring, + uint8_t **data, uint32_t size) { ring_buf_idx_t head_offset, wrap_size; @@ -47,7 +54,8 @@ uint32_t ring_buf_area_claim(struct ring_buf *buf, struct ring_buf_index *ring, return size; } -int ring_buf_area_finish(struct ring_buf *buf, struct ring_buf_index *ring, uint32_t size) +int K_ISR_SAFE ring_buf_area_finish(struct ring_buf *buf, struct ring_buf_index *ring, + uint32_t size) { ring_buf_idx_t claimed_size, tail_offset; @@ -68,7 +76,7 @@ int ring_buf_area_finish(struct ring_buf *buf, struct ring_buf_index *ring, uint return 0; } -uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size) +uint32_t K_ISR_SAFE ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size) { uint8_t *dst; uint32_t partial_size; @@ -87,13 +95,16 @@ uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size) } while (size != 0); err = ring_buf_put_finish(buf, total_size); - __ASSERT_NO_MSG(err == 0); - ARG_UNUSED(err); + if (unlikely(err != 0)) { + /* finish of our own claim cannot fail unless the index + * state was corrupted by unserialized access */ + k_panic(); + } return total_size; } -uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size) +uint32_t K_ISR_SAFE ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size) { uint8_t *src; uint32_t partial_size; @@ -114,25 +125,33 @@ uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size) } while (size != 0); err = ring_buf_get_finish(buf, total_size); - __ASSERT_NO_MSG(err == 0); - ARG_UNUSED(err); + if (unlikely(err != 0)) { + /* finish of our own claim cannot fail unless the index + * state was corrupted by unserialized access */ + k_panic(); + } return total_size; } -uint32_t ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size) +uint32_t K_ISR_SAFE ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size) { uint8_t *src; uint32_t partial_size; uint32_t total_size = 0U; int err; + /* checked before the loop so the doc contract ("Cannot be NULL") + * is enforced on an empty buffer too, not only once data flows */ + if (unlikely(data == NULL)) { + k_panic(); + } + do { partial_size = ring_buf_get_claim(buf, &src, size); if (partial_size == 0) { break; } - __ASSERT_NO_MSG(data != NULL); memcpy(data, src, partial_size); data += partial_size; total_size += partial_size; @@ -141,8 +160,9 @@ uint32_t ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size) /* effectively unclaim total_size bytes */ err = ring_buf_get_finish(buf, 0); - __ASSERT_NO_MSG(err == 0); - ARG_UNUSED(err); + if (unlikely(err != 0)) { + k_panic(); + } return total_size; } From f440e8db2dbe07a4b86934cf2922ae8cf466a138 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Wed, 10 Jun 2026 13:44:04 -0600 Subject: [PATCH 7/9] docs: ring_buf review feedback -- concurrency contract, claim notes, retvals Doc-only fixes from review (code unchanged, upstream-verbatim): - Concurrency note rewritten: it overclaimed "lock-free SPSC" without stating that no field is volatile and no barriers are issued. Now documents that the query inlines read one index from each side (tear-free aligned loads, conservatively stale), forbids busy-waiting on them (the compiler may hoist the load out of a call-free loop), and requires per-handoff k_sem pairing on SMP. Also notes the implementation is K_ISR_SAFE. - RING_BUF_INIT: @warning that size8 > RING_BUFFER_MAX_SIZE is NOT checked on this path (unlike RING_BUF_DECLARE/ring_buf_init) and silently corrupts; macro body stays upstream-verbatim. - put_claim/get_claim/peek: @notes that finishing rewinds head to tail, so an outstanding claim must be finished before any other same-side call (including the copy-mode functions); restored upstream's dropped "with a non-zero `size`" qualifier on the peek doc. - put_finish/get_finish: -EINVAL condition corrected -- the code (here and upstream) rejects size > outstanding claimed bytes, not "exceeds free space"/"valid bytes" as upstream's doc has always misstated. - size_get: adopted current upstream main's "available data" wording (v3.7's "used space" miscounts outstanding claims); three @retval-with-prose fixed to @return (also fixed upstream post-v3.7). - Removed seven @warnings referencing the unported ring_buf_item_ API; RING_BUF_DECLARE gains a file-scope-only @note. - Kconfig: RING_BUFFER_LARGE costs 12 extra bytes per struct ring_buf on the 32-bit target (6 index fields x 2 bytes; measured 20 -> 32), not 6 as the help text claimed. Co-Authored-By: Claude Fable 5 --- components/zkernel/Kconfig | 6 +- .../include/boreas/zephyr/sys/ring_buffer.h | 124 +++++++++++------- 2 files changed, 79 insertions(+), 51 deletions(-) diff --git a/components/zkernel/Kconfig b/components/zkernel/Kconfig index 59a3e59..ed1b15c 100644 --- a/components/zkernel/Kconfig +++ b/components/zkernel/Kconfig @@ -6,8 +6,10 @@ menu "Boreas Kernel (zkernel)" help Widens the ring buffer index type from uint16_t to uint32_t, raising the maximum ring_buf capacity from 32767 bytes to - ~2 GiB at the cost of 6 extra bytes per struct ring_buf. - Mirrors upstream Zephyr's CONFIG_RING_BUFFER_LARGE. + ~2 GiB at the cost of 12 extra bytes per struct ring_buf + (six index fields widen by 2 bytes each; 20 -> 32 bytes on + the 32-bit target). Mirrors upstream Zephyr's + CONFIG_RING_BUFFER_LARGE. config ZKERNEL_MUTEX_DEBUG bool "Enable mutex lock-ordering assertions" diff --git a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h index 7bdb6dc..3f87edc 100644 --- a/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h +++ b/components/zkernel/include/boreas/zephyr/sys/ring_buffer.h @@ -8,20 +8,34 @@ * lib/utils/ring_buffer.c -- both Apache-2.0), adapted only for the * Boreas include layout and toolchain shims (see sys/util.h). * - * Divergence: the upstream item-mode API (ring_buf_item_put/_get, - * ring_buf_item_init, RING_BUF_ITEM_DECLARE*) is intentionally NOT - * ported -- it is @deprecated upstream ("use ") - * and the byte-mode API below is what the UART IRQ pattern, the - * direct-UART shell transport, and the modbus serial backend use. - * - * Concurrency (unchanged from upstream): the buffer is lock-free for a - * SINGLE producer and a SINGLE consumer in separate contexts (two - * threads, or thread + ISR) -- the producer touches only `put` (and - * reads `get.tail`); the consumer touches only `get` (and reads - * `put.tail`). Multiple producers OR multiple consumers must serialize - * access externally (e.g. a k_mutex or by locking interrupts). On SMP, - * pair the buffer with a k_sem to establish ordering between producer - * and consumer. + * Divergences: + * - the upstream item-mode API (ring_buf_item_put/_get, + * ring_buf_item_init, RING_BUF_ITEM_DECLARE*) is intentionally NOT + * ported -- it is @deprecated upstream ("use ") + * and the byte-mode API below is what the UART IRQ pattern, the + * direct-UART shell transport, and the modbus serial backend use; + * - the implementation functions are K_ISR_SAFE (IRAM-resident) so + * the byte API is callable from ESP_INTR_FLAG_IRAM ISRs -- see + * src/ring_buf.c. + * + * Concurrency (code unchanged from upstream): the buffer is lock-free + * for a SINGLE producer and a SINGLE consumer in separate contexts + * (two threads, or thread + ISR) -- the producer writes only `put`, + * the consumer writes only `get`, and the query inlines (is_empty, + * size_get, space_get) read one index from each side as aligned + * single-word loads, so they are tear-free from either side and return + * conservatively stale answers. No field is volatile and no memory + * barriers are issued, so: + * - do NOT busy-wait on the query inlines: in a call-free loop the + * compiler may legally hoist the index load and never observe the + * other side's progress. Block on a k_sem/k_event signaled by the + * other side instead; + * - on SMP, the buffer alone does not order the data bytes against + * the index publication: pair each handoff with a k_sem (or other + * acquire/release primitive) and do not drain past the handoff + * you were signaled for. + * Multiple producers OR multiple consumers must serialize access + * externally (e.g. a k_mutex or by locking interrupts). */ #pragma once @@ -99,6 +113,13 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t * For use in a struct ring_buf initializer with a caller-provided data * area, e.g. `struct ring_buf rb = RING_BUF_INIT(buf, sizeof(buf));`. * + * @warning @a size8 must be <= RING_BUFFER_MAX_SIZE. Unlike + * @ref RING_BUF_DECLARE (BUILD_ASSERT) and @ref ring_buf_init + * (runtime __ASSERT), an initializer cannot check this; an oversized + * buffer compiles silently and corrupts data without error once + * cumulative traffic exceeds the index range (upstream has the same + * unchecked macro). + * * @param buf Pointer to the data area (uint8_t[]). * @param size8 Size of the data area (in bytes). */ @@ -119,6 +140,11 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t * * @code extern struct ring_buf ; @endcode * + * @note File-scope only: the macro expands to a BUILD_ASSERT plus two + * declarations, so it cannot be prefixed with `static`, and at block + * scope it would silently create a fresh automatic struct ring_buf per + * call over one shared function-static data array. + * * @param name Name of the ring buffer. * @param size8 Size of ring buffer (in bytes). */ @@ -131,7 +157,7 @@ static inline void ring_buf_internal_reset(struct ring_buf *buf, ring_buf_idx_t * @brief Initialize a ring buffer for byte data. * * This routine initializes a ring buffer, prior to its first use. It is only - * used for the byte data, for which the size is expressed in bytes. + * used for ring buffers not defined using RING_BUF_DECLARE. * * @param buf Address of ring buffer. * @param size Ring buffer size (in bytes). @@ -195,11 +221,16 @@ static inline uint32_t ring_buf_capacity_get(const struct ring_buf *buf) } /** - * @brief Determine used space in a ring buffer. + * @brief Determine size of available data in a ring buffer. + * + * Counts committed-and-unread bytes only: bytes inside an outstanding + * (unfinished) claim are counted neither here nor by + * @ref ring_buf_space_get. (Wording from current upstream main; the + * v3.7-era "used space" phrasing was misleading around claims.) * * @param buf Address of ring buffer. * - * @return Ring buffer space used (in bytes). + * @return Ring buffer data size (in bytes). */ static inline uint32_t ring_buf_size_get(const struct ring_buf *buf) { @@ -220,9 +251,10 @@ static inline uint32_t ring_buf_size_get(const struct ring_buf *buf) * concurrent write operations, either by preventing all writers from * being preempted or by using a mutex to govern writes to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). + * @note An outstanding claim must be completed with + * @ref ring_buf_put_finish before any other put-side call (including + * copy-mode @ref ring_buf_put): finishing rewinds the allocation head to + * the committed tail, silently discarding whatever was still claimed. * * @param buf Address of ring buffer. * @param data Pointer to the address. It is set to a location within @@ -251,15 +283,14 @@ static inline uint32_t ring_buf_put_claim(struct ring_buf *buf, uint8_t **data, * concurrent write operations, either by preventing all writers from * being preempted or by using a mutex to govern writes to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). - * * @param buf Address of ring buffer. * @param size Number of valid bytes in the allocated buffers. * * @retval 0 Successful operation. - * @retval -EINVAL Provided @a size exceeds free space in the ring buffer. + * @retval -EINVAL Provided @a size exceeds the bytes claimed by preceding + * @ref ring_buf_put_claim invocations and not yet finished. + * (Upstream documents "exceeds free space", but the code -- here + * and upstream -- checks the outstanding claimed amount.) */ static inline int ring_buf_put_finish(struct ring_buf *buf, uint32_t size) { @@ -276,15 +307,11 @@ static inline int ring_buf_put_finish(struct ring_buf *buf, uint32_t size) * concurrent write operations, either by preventing all writers from * being preempted or by using a mutex to govern writes to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). - * * @param buf Address of ring buffer. * @param data Address of data. * @param size Data size (in bytes). * - * @retval Number of bytes written. + * @return Number of bytes written. */ uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size); @@ -300,9 +327,11 @@ uint32_t ring_buf_put(struct ring_buf *buf, const uint8_t *data, uint32_t size); * concurrent read operations, either by preventing all readers from being * preempted or by using a mutex to govern reads to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). + * @note An outstanding claim must be completed with + * @ref ring_buf_get_finish before any other get-side call (including + * copy-mode @ref ring_buf_get and @ref ring_buf_peek): finishing rewinds + * the claim head to the freed tail, silently discarding whatever was + * still claimed. * * @param buf Address of ring buffer. * @param data Pointer to the address. It is set to a location within @@ -331,15 +360,14 @@ static inline uint32_t ring_buf_get_claim(struct ring_buf *buf, uint8_t **data, * concurrent read operations, either by preventing all readers from being * preempted or by using a mutex to govern reads to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). - * * @param buf Address of ring buffer. * @param size Number of bytes that can be freed. * * @retval 0 Successful operation. - * @retval -EINVAL Provided @a size exceeds valid bytes in the ring buffer. + * @retval -EINVAL Provided @a size exceeds the bytes claimed by preceding + * @ref ring_buf_get_claim invocations and not yet finished. + * (Upstream documents "exceeds valid bytes", but the code -- here + * and upstream -- checks the outstanding claimed amount.) */ static inline int ring_buf_get_finish(struct ring_buf *buf, uint32_t size) { @@ -356,15 +384,11 @@ static inline int ring_buf_get_finish(struct ring_buf *buf, uint32_t size) * concurrent read operations, either by preventing all readers from being * preempted or by using a mutex to govern reads to the ring buffer. * - * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). - * * @param buf Address of ring buffer. * @param data Address of the output buffer. Can be NULL to discard data. * @param size Data size (in bytes). * - * @retval Number of bytes written to the output buffer. + * @return Number of bytes written to the output buffer. */ uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size); @@ -380,19 +404,21 @@ uint32_t ring_buf_get(struct ring_buf *buf, uint8_t *data, uint32_t size); * preempted or by using a mutex to govern reads to the ring buffer. * * @warning - * Ring buffer instance should not mix byte access and item access - * (calls prefixed with ring_buf_item_). - * - * @warning * Multiple calls to peek will result in the same data being 'peeked' multiple * times. To remove data, use either @ref ring_buf_get or @ref - * ring_buf_get_claim followed by @ref ring_buf_get_finish. + * ring_buf_get_claim followed by @ref ring_buf_get_finish with a non-zero + * `size`. + * + * @note Internally peek claims and then unclaims (finishes with size 0), + * which rewinds the get-side claim head -- so it must not be called while + * a @ref ring_buf_get_claim is outstanding (the claim would be silently + * voided and peek would return bytes past it). * * @param buf Address of ring buffer. * @param data Address of the output buffer. Cannot be NULL. * @param size Data size (in bytes). * - * @retval Number of bytes written to the output buffer. + * @return Number of bytes written to the output buffer. */ uint32_t ring_buf_peek(struct ring_buf *buf, uint8_t *data, uint32_t size); From 0445556b59816a9fd826639a5401c095339a4488 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Wed, 10 Jun 2026 13:44:20 -0600 Subject: [PATCH 8/9] test: ring_buf -- seeded index-wrap and peek-across-wrap coverage Two coverage gaps closed: - test_ring_buf_index_wrap_seeded: the traffic-volume stress tests can only reach the default uint16_t 65536 wrap; under CONFIG_RING_BUFFER_LARGE the 2^32 wrap was silently untested in exactly the config the option exists for. Seeding the indices just below the wrap via ring_buf_internal_reset (the technique upstream's ringbuffer test suite uses) makes the full/empty discrimination coverage index-width-independent. Verified locally: full suite passes on the linux target with CONFIG_RING_BUFFER_LARGE=y. - test_ring_buf_peek_across_wrap_multi_claim_get: peek was only tested contiguous-at-offset-0; now covers peek walking both physical segments of wrapped data and rewinding via its internal get_finish(0), plus multiple get-side claims completed by a single finish. Cleanups: per-byte TEST_ASSERT_EQUAL_UINT8 loops (~300k Unity calls across the two stress tests) replaced with per-chunk TEST_ASSERT_EQUAL_UINT8_ARRAY (better failure diagnostics: reports the element index); put_claim_wraps now drains and verifies its patterned writes landed at the correct physical offsets instead of leaving them unchecked; NULL-discard priming replaces drain-into-out + memset; dead sequence fill dropped from the saturates test. Co-Authored-By: Claude Fable 5 --- test/main/test_ring_buf.c | 129 +++++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 22 deletions(-) diff --git a/test/main/test_ring_buf.c b/test/main/test_ring_buf.c index 574de7f..cca59a7 100644 --- a/test/main/test_ring_buf.c +++ b/test/main/test_ring_buf.c @@ -54,11 +54,9 @@ static void test_ring_buf_put_saturates_at_capacity(void) { uint8_t storage[8]; struct ring_buf rb; - uint8_t in[12]; + /* contents never inspected -- this test asserts only counts */ + uint8_t in[12] = {0}; - for (int i = 0; i < (int)sizeof(in); i++) { - in[i] = (uint8_t)i; - } ring_buf_init(&rb, sizeof(storage), storage); /* Only capacity bytes fit; the rest is refused. */ @@ -256,6 +254,13 @@ static void test_ring_buf_put_claim_wraps(void) } TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, first + second)); TEST_ASSERT_EQUAL(8, ring_buf_size_get(&rb)); + + /* Drain and verify the two claims landed at the correct physical + * offsets (end segment first, then the wrapped start). */ + const uint8_t expect[8] = {0xAA, 0xBB, 0xC0, 0xC1, 0xC2, 0xC3, 0xC4, 0xC5}; + uint8_t out[8] = {0}; + TEST_ASSERT_EQUAL(8, ring_buf_get(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(expect, out, sizeof(expect)); } /* ring_buf_put/get themselves loop over the wrap, so a single call that @@ -270,9 +275,9 @@ static void test_ring_buf_put_get_across_wrap(void) ring_buf_init(&rb, sizeof(storage), storage); - /* Push the offset forward by 5, drain it. */ + /* Push the offset forward by 5, discard it. */ ring_buf_put(&rb, a, sizeof(a)); - ring_buf_get(&rb, out, sizeof(a)); + TEST_ASSERT_EQUAL(5, ring_buf_get(&rb, NULL, sizeof(a))); /* This 6-byte write starts at offset 5 and wraps the 8-byte buffer. */ TEST_ASSERT_EQUAL(6, ring_buf_put(&rb, b, sizeof(b))); @@ -281,7 +286,6 @@ static void test_ring_buf_put_get_across_wrap(void) TEST_ASSERT_EQUAL(6, ring_buf_size_get(&rb)); TEST_ASSERT_EQUAL(2, ring_buf_space_get(&rb)); - memset(out, 0, sizeof(out)); TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, out, sizeof(b))); TEST_ASSERT_EQUAL_UINT8_ARRAY(b, out, sizeof(b)); /* Fully drained after the wrap. */ @@ -296,12 +300,13 @@ static void test_ring_buf_many_wrap_cycles(void) uint8_t storage[7]; /* non-power-of-two on purpose */ struct ring_buf rb; uint8_t next_write = 0; - uint8_t next_read = 0; ring_buf_init(&rb, sizeof(storage), storage); - /* Far more bytes than fit in ring_buf_idx_t at a time, streamed in - * 5-byte chunks through a 7-byte buffer so the offset wraps often. */ + /* 100000 bytes streamed in 5-byte chunks through a 7-byte buffer so + * the physical offset wraps constantly and the default uint16_t + * indices cross their 65536 range ~1.5 times (the seeded test below + * covers the index wrap under CONFIG_RING_BUFFER_LARGE). */ for (int cycle = 0; cycle < 20000; cycle++) { uint8_t chunk[5]; @@ -310,26 +315,27 @@ static void test_ring_buf_many_wrap_cycles(void) } TEST_ASSERT_EQUAL(5, ring_buf_put(&rb, chunk, sizeof(chunk))); + /* drained fully each cycle, so the chunk is the expectation */ uint8_t out[5] = {0}; TEST_ASSERT_EQUAL(5, ring_buf_get(&rb, out, sizeof(out))); - for (int i = 0; i < 5; i++) { - TEST_ASSERT_EQUAL_UINT8(next_read++, out[i]); - } + TEST_ASSERT_EQUAL_UINT8_ARRAY(chunk, out, sizeof(out)); } TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); } /* Fill-to-full / drain-to-empty repeatedly, enough cycles to carry the - * uint16_t indices across their 65536 wrap, asserting the full and - * empty discriminations AT high `allocated` each cycle. The + * default uint16_t indices across their 65536 wrap, asserting the full + * and empty discriminations AT high `allocated` each cycle. The * many-wrap-cycles test above drains within a 7-byte span so allocated * never approaches capacity; this one holds the buffer completely full * across the index wrap, exercising space_get == 0 / put returning 0 * (full) vs is_empty (empty) right where the modular subtraction must - * stay unambiguous. */ + * stay unambiguous. (Traffic volume cannot reach the 2^32 wrap under + * CONFIG_RING_BUFFER_LARGE -- the seeded test below covers that.) */ static void test_ring_buf_full_empty_across_wrap(void) { static uint8_t storage[4096]; + static uint8_t expected[sizeof(storage)]; /* per-cycle ramp, off-stack */ struct ring_buf rb; uint8_t *dst; @@ -340,14 +346,16 @@ static void test_ring_buf_full_empty_across_wrap(void) for (int cycle = 0; cycle < 40; cycle++) { uint8_t seed = (uint8_t)cycle; + for (uint32_t i = 0; i < sizeof(storage); i++) { + expected[i] = (uint8_t)(seed + i); + } + /* Fill to exactly full via the claim path. */ uint32_t total = 0; while (total < sizeof(storage)) { uint32_t n = ring_buf_put_claim(&rb, &dst, sizeof(storage) - total); TEST_ASSERT_NOT_EQUAL(0, n); - for (uint32_t i = 0; i < n; i++) { - dst[i] = (uint8_t)(seed + total + i); - } + memcpy(dst, &expected[total], n); total += n; TEST_ASSERT_EQUAL(0, ring_buf_put_finish(&rb, n)); } @@ -364,9 +372,7 @@ static void test_ring_buf_full_empty_across_wrap(void) uint8_t *src; uint32_t n = ring_buf_get_claim(&rb, &src, sizeof(storage) - got); TEST_ASSERT_NOT_EQUAL(0, n); - for (uint32_t i = 0; i < n; i++) { - TEST_ASSERT_EQUAL_UINT8((uint8_t)(seed + got + i), src[i]); - } + TEST_ASSERT_EQUAL_UINT8_ARRAY(&expected[got], src, n); got += n; TEST_ASSERT_EQUAL(0, ring_buf_get_finish(&rb, n)); } @@ -378,6 +384,83 @@ static void test_ring_buf_full_empty_across_wrap(void) } } +/* The full/empty discrimination must also hold while head/tail/base + * cross the numeric wrap of ring_buf_idx_t itself, whatever its width. + * Traffic volume can only reach the default uint16_t wrap, so this test + * seeds the indices just below the wrap via ring_buf_internal_reset + * (whose doc blesses non-zero values for validation testing -- the same + * technique upstream's ringbuffer test suite uses), making the coverage + * independent of CONFIG_RING_BUFFER_LARGE. */ +static void test_ring_buf_index_wrap_seeded(void) +{ + uint8_t storage[64]; + struct ring_buf rb; + uint8_t chunk[64]; + uint8_t out[64]; + + ring_buf_init(&rb, sizeof(storage), storage); + /* Two full buffer-loads below the wrap: the indices land exactly on + * 0 at the end of cycle 2, so cycles 3-4 exercise post-wrap state. */ + ring_buf_internal_reset(&rb, (ring_buf_idx_t)(0U - 2U * sizeof(storage))); + + for (int cycle = 0; cycle < 4; cycle++) { + for (uint32_t i = 0; i < sizeof(chunk); i++) { + chunk[i] = (uint8_t)(cycle + i); + } + TEST_ASSERT_EQUAL(sizeof(storage), ring_buf_put(&rb, chunk, sizeof(chunk))); + + /* Full at (and across) the index wrap. */ + TEST_ASSERT_EQUAL(0, ring_buf_space_get(&rb)); + TEST_ASSERT_FALSE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(0, ring_buf_put(&rb, chunk, 1)); + + memset(out, 0, sizeof(out)); + TEST_ASSERT_EQUAL(sizeof(storage), ring_buf_get(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(chunk, out, sizeof(chunk)); + + /* Empty at (and across) the index wrap. */ + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(sizeof(storage), ring_buf_space_get(&rb)); + } +} + +/* peek must walk both physical segments of wrapped data (two internal + * claim iterations) and then rewind the claim head back across the + * physical end via its internal get_finish(0); the get side must + * likewise allow several claims before one finish covering all of + * them. Neither path is reachable through the contiguous tests above. */ +static void test_ring_buf_peek_across_wrap_multi_claim_get(void) +{ + uint8_t storage[8]; + struct ring_buf rb; + const uint8_t fill[] = {1, 2, 3, 4, 5, 6}; + const uint8_t wrapped[] = {0xD0, 0xD1, 0xD2, 0xD3, 0xD4, 0xD5}; + uint8_t out[8] = {0}; + uint8_t *src; + + ring_buf_init(&rb, sizeof(storage), storage); + + /* Park the offset at 6 so the next 6 bytes span the physical end. */ + ring_buf_put(&rb, fill, sizeof(fill)); + TEST_ASSERT_EQUAL(6, ring_buf_get(&rb, NULL, sizeof(fill))); + TEST_ASSERT_EQUAL(6, ring_buf_put(&rb, wrapped, sizeof(wrapped))); + + /* peek the wrapped data: both segments, in order, not consumed. */ + TEST_ASSERT_EQUAL(6, ring_buf_peek(&rb, out, sizeof(out))); + TEST_ASSERT_EQUAL_UINT8_ARRAY(wrapped, out, sizeof(wrapped)); + TEST_ASSERT_EQUAL(6, ring_buf_size_get(&rb)); + + /* Drain with two claims (split by the physical end) and a single + * finish covering both. */ + TEST_ASSERT_EQUAL(2, ring_buf_get_claim(&rb, &src, 8)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(&wrapped[0], src, 2); + TEST_ASSERT_EQUAL(4, ring_buf_get_claim(&rb, &src, 8)); + TEST_ASSERT_EQUAL_UINT8_ARRAY(&wrapped[2], src, 4); + TEST_ASSERT_EQUAL(0, ring_buf_get_finish(&rb, 6)); + TEST_ASSERT_TRUE(ring_buf_is_empty(&rb)); + TEST_ASSERT_EQUAL(8, ring_buf_space_get(&rb)); +} + /* ---------------------------------------------------------------- * RING_BUF_DECLARE static instance * ---------------------------------------------------------------- */ @@ -414,5 +497,7 @@ void test_ring_buf_group(void) RUN_TEST(test_ring_buf_put_get_across_wrap); RUN_TEST(test_ring_buf_many_wrap_cycles); RUN_TEST(test_ring_buf_full_empty_across_wrap); + RUN_TEST(test_ring_buf_index_wrap_seeded); + RUN_TEST(test_ring_buf_peek_across_wrap_multi_claim_get); RUN_TEST(test_ring_buf_declare_usable); } From 9ef275f12b516cda4e5188db1210a8f734df4de5 Mon Sep 17 00:00:00 2001 From: swoiszwillo Date: Wed, 10 Jun 2026 14:05:30 -0600 Subject: [PATCH 9/9] docs: note ALWAYS_INLINE is the exception to toolchain.h's ifndef rule (Copilot) The file-level comment claimed all definitions are #ifndef-guarded, but ALWAYS_INLINE is intentionally #undef'd and redefined unconditionally (the RISC-V -Werror=attributes fix). Call out the exception so the include-order contract isn't misleading. Co-Authored-By: Claude Opus 4.8 (1M context) --- components/zkernel/include/boreas/zephyr/toolchain.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/zkernel/include/boreas/zephyr/toolchain.h b/components/zkernel/include/boreas/zephyr/toolchain.h index 07f085c..faa15a5 100644 --- a/components/zkernel/include/boreas/zephyr/toolchain.h +++ b/components/zkernel/include/boreas/zephyr/toolchain.h @@ -4,8 +4,10 @@ * * Zephyr-compatible toolchain shims (upstream * spellings), kept thin so near-verbatim ports can keep their upstream - * #include shape. All definitions are #ifndef-guarded: a TU that - * already got a definition elsewhere keeps the first one it saw. + * #include shape. Definitions are #ifndef-guarded -- a TU that already + * got a definition elsewhere keeps the first one it saw -- EXCEPT + * ALWAYS_INLINE, which is intentionally #undef'd and redefined + * unconditionally (see the note on it below). */ #pragma once