From 948aeab8251bda0eef86bfc0861f187330c3407d Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:00:38 +0100 Subject: [PATCH 1/8] clang-format: Short loops on single line --- .clang-format | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-format b/.clang-format index 38abf14..f98fb9c 100644 --- a/.clang-format +++ b/.clang-format @@ -51,7 +51,7 @@ AllowShortEnumsOnASingleLine: true AllowShortFunctionsOnASingleLine: All AllowShortIfStatementsOnASingleLine: Never AllowShortLambdasOnASingleLine: All -AllowShortLoopsOnASingleLine: false +AllowShortLoopsOnASingleLine: true AlwaysBreakAfterDefinitionReturnType: All AlwaysBreakAfterReturnType: AllDefinitions AlwaysBreakBeforeMultilineStrings: false From 4a90d24069544f518d8f06f0eba2ec7bd4c7841c Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:01:55 +0100 Subject: [PATCH 2/8] Makefile: debug is default target, make sure include path is set to project root, better clean --- Makefile | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 10c26a0..ad23202 100644 --- a/Makefile +++ b/Makefile @@ -1,19 +1,21 @@ # SPDX-License-Identifier: MIT CC = gcc -CFLAGS = -Wall -Wextra -Werror -Wno-unused-parameter -CFLAGS += -Wno-unused-variable -Wno-unused-function +CFLAGS = -Wall -Wextra -Werror -Wno-unused-parameter +CFLAGS += -Wno-unused-variable -Wno-unused-function CFLAGS += -Wno-unused-but-set-variable -Wno-unused-value -Wno-unused-label CFLAGS += -Wno-unused-result -Wno-unused-const-variable -ifneq ($(DEBUG), 1) +CFLAGS += -I. + +ifneq ($(RELEASE), 1) +CFLAGS += -g -O0 -std=c99 -march=native -mtune=native +CFLAGS += -DDEBUG +else CFLAGS += -fno-exceptions -fno-asynchronous-unwind-tables -fno-ident CFLAGS += -fno-unwind-tables -fno-stack-protector -fno-plt -fno-pic CFLAGS += -O3 -std=c99 -march=native -mtune=native -fomit-frame-pointer CFLAGS += -fshort-enums -else -CFLAGS += -g -O0 -std=c99 -march=native -mtune=native -CFLAGS += -DDEBUG endif # DEBUG LDFLAGS = -lm @@ -50,7 +52,7 @@ install: @cp ringbuf.h /usr/local/include clean: - rm -f $(OBJECTS) $(ASMS) driver librbuf.a librbuf.so + rm -f $(OBJECTS) $(ASMS) driver librbuf.a librbuf.so **/*.o **/*.elf asm: $(ASMS) $(OBJECTS) wc -l $(ASMS) From 20b6e5c2fdb6e3ae2eb5403aee256ad51f12fa46 Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:02:49 +0100 Subject: [PATCH 3/8] Prevent double-free in rb_destroy --- ringbuf.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ringbuf.c b/ringbuf.c index d73affe..ffc979e 100644 --- a/ringbuf.c +++ b/ringbuf.c @@ -29,7 +29,18 @@ rb_init(struct RingBuf *rb, rb_size_t capacity, ALLOC_T malloc_fn, void rb_destroy(struct RingBuf *rb, FREE_T free_fn) { - free_fn(rb->buffer); + if(rb->buffer) { // Prevent double-free + free_fn(rb->buffer); + rb->buffer = NULL; + rb->buffer_end = NULL; + + rb->struct_size = 0; + rb->capacity = 0; + rb->count = 0; + + rb->write_head = NULL; + rb->read_head = NULL; + } } void @@ -46,6 +57,8 @@ rb_push_back(struct RingBuf *rb, const void *item, MEMCPY_T memcpy_fn) if(rb->count == rb->capacity) return Full; + assert(rb->buffer != NULL); + memcpy_fn(rb->write_head, item, rb->struct_size); // Advance the write head From 3308e61dc1f74c4723a52494328d099461bfa027 Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:03:11 +0100 Subject: [PATCH 4/8] Missing include from previous commit --- ringbuf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ringbuf.c b/ringbuf.c index ffc979e..dc2078f 100644 --- a/ringbuf.c +++ b/ringbuf.c @@ -4,6 +4,7 @@ // #include #include "ringbuf.h" +#include #include #ifdef DEBUG From 259c048bd5cb2ed2ebecae49267f7e809139ff2f Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:03:47 +0100 Subject: [PATCH 5/8] Move certain debug related functionality into header/source, feature gated by DEBUG flag --- driver.c | 21 +++++---------------- ringbuf.c | 17 +++++++++++++++++ ringbuf.h | 13 +++++++++++++ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/driver.c b/driver.c index 58045ed..19d0e74 100644 --- a/driver.c +++ b/driver.c @@ -4,27 +4,16 @@ #include #include -#define rb_size_t size_t +#ifndef DEBUG +#define DEBUG +#endif + +// #define rb_size_t size_t // #define rb_size_t int #include "ringbuf.h" typedef int DATATYPE; -/** - * @brief Debug print and empty the ringbuf - */ -void -rb_debug_empty(struct RingBuf *rb) -{ - int d; - if(rb->count == 0) - return; - printf("Debug Data: ["); - while(rb_pop_front(rb, &d, memcpy) == ReadOk) - printf("%d,", d); - printf("\b]\n"); -} - int main(void) { diff --git a/ringbuf.c b/ringbuf.c index dc2078f..59626f7 100644 --- a/ringbuf.c +++ b/ringbuf.c @@ -131,6 +131,21 @@ rb_pop_front(struct RingBuf *rb, void *item, MEMCPY_T memcpy_fn) return ReadOk; } +#ifdef DEBUG +#include + +void +rb_debug_empty(struct RingBuf *rb) +{ + int d; + if(rb->count == 0) + return; + printf("Debug Data: ["); + while(rb_pop_front(rb, &d, memcpy) == ReadOk) + printf("%d,", d); + printf("\b]\n"); +} + void rb_debug_print(struct RingBuf *rb) { @@ -147,3 +162,5 @@ rb_debug_print(struct RingBuf *rb) DEBUG_PRINT("============%s\n", ""); } + +#endif diff --git a/ringbuf.h b/ringbuf.h index 46db542..1844437 100644 --- a/ringbuf.h +++ b/ringbuf.h @@ -111,12 +111,25 @@ enum ReadResult rb_pop_front(struct RingBuf *rb, void *item, /** * @brief Free the ring buffer + * + * @details This function is idempotent, consecutive calls will not result in a + * double free. + * * @param rb The ring buffer * @param free The free function */ void rb_destroy(struct RingBuf *rb, void(free)()); +#ifdef DEBUG + /** * @brief Debug print */ void rb_debug_print(struct RingBuf *rb); + +/** + * @brief Debug print and empty the ringbuf + */ +void rb_debug_empty(struct RingBuf *rb); + +#endif From 4fab8ca207199b8c0f14dea208163e10c3b0408f Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:03:57 +0100 Subject: [PATCH 6/8] Extended readme --- README.md | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 0000000..1626f2c --- /dev/null +++ b/README.md @@ -0,0 +1,44 @@ +# RingBuf + +RingBuf is an allocator-agnostic, non-overwriting circular/ring buffer +implementation in C99. +See: [Circular Buffer (Wikipedia)](https://en.wikipedia.org/wiki/Circular_buffer) + +## Features +- Space Efficiency +The code is designed to be portable and flexible. The inspiration initially +came to me when designing a network driver. When operating in memory +constrained environments, every byte is of upmost importance. Traditional +metaprogramming such as templating in C++ and template metaprogramming in C, +although generic has the side effect of expanding into discrete machine code +specific for the specialization applied, essentially scaling (spatially) linear +with the amount of different datatypes we specialize on. This implementation +circumvents this by treating all data as a void pointer with a length. This +implies **no deep copies**. + +- Allocator Agnostic +Another common characteristic of memory constrained environments are +custom malloc implementations and/or variations. + - Signatures + - Arena + +## Design considerations +- Holding a reference to malloc internally would make a tidier interface +- Passing a user-defined function for deep-copies would enable certain applications + +## Usage +In essence: +```c +int value = 42; +struct RingBuf rb; +rb_init(&rb, 10, malloc, sizeof(int)); +rb_push_back(&rb, (void *)&data, memcpy); +rb_pop_front(&rb, &value, memcpy); +``` +Most of these functions return Enum result types. See: +[ringbuf.h](./ringbuf.h). + +## Future Improvements +- Trim the code +- Reduce boilerplate in tests +- Reduce the number of tests in exchange for better test fit From 1e760664005072829c367b38562177955f7d5cbb Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:04:23 +0100 Subject: [PATCH 7/8] gitignore for chache, json and .elf --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 32aec77..f25f439 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ *.a build driver +*.elf +*.json +.cache From bfffbb92115ba4058281306008636a18e0359b96 Mon Sep 17 00:00:00 2001 From: Imbus <> Date: Wed, 25 Dec 2024 14:05:22 +0100 Subject: [PATCH 8/8] Basic tests implemented using cmocka --- Makefile | 8 +++- test/test_ringbuf.c | 101 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 test/test_ringbuf.c diff --git a/Makefile b/Makefile index ad23202..50d4220 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,9 @@ ASMS = $(C_SOURCES:.c=.s) all: $(OBJECTS) +test: test/test.elf + ./test/test.elf + %.o: %.c $(C_HEADERS) @echo "CC $<" @$(CC) $(CFLAGS) -c $< -o $@ @@ -41,6 +44,9 @@ driver: $(OBJECTS) run: driver @./driver +test/test.elf: test/test_ringbuf.o ringbuf.o + @$(CC) $(CFLAGS) $^ -o $@ -lpthread -lcmocka + lib: $(OBJECTS) @ar rcs librbuf.a ringbuf.o @@ -64,4 +70,4 @@ tidy: format: clang-format -i $(C_SOURCES) $(C_HEADERS) -.PHONY: all clean format asm +.PHONY: all clean format asm test diff --git a/test/test_ringbuf.c b/test/test_ringbuf.c new file mode 100644 index 0000000..44756e9 --- /dev/null +++ b/test/test_ringbuf.c @@ -0,0 +1,101 @@ +#include +#include +#include +#include +#include +#include +#include "ringbuf.h" + +/** Tests initialization */ +static void +test_rb_init_empty(void **state) { + struct RingBuf rb; + rb_size_t capacity = 10; + rb_size_t struct_size = sizeof(int); + + rb_init(&rb, capacity, malloc, struct_size); + + assert_int_equal(rb.capacity, capacity); + assert_int_equal(rb.count, 0); +} + +/** Tests push_back */ +static void +test_rb_push_back(void **state) { + struct RingBuf rb; + rb_size_t capacity = 10; + rb_size_t struct_size = sizeof(int); + + rb_init(&rb, capacity, malloc, struct_size); + + int data = 10; + + enum WriteResult wr = rb_push_back(&rb, (void *)&data, memcpy); + assert_int_equal(wr, WriteOk); + + assert_int_equal(rb.capacity, capacity); + assert_int_equal(rb.count, 1); +} + +/** Tests the destroy function */ +static void +test_rb_init_destroy(void **state) { + struct RingBuf rb = { 0, 0, 0, NULL, NULL, NULL, NULL }; + rb_size_t capacity = 10; + rb_size_t struct_size = sizeof(int); + + rb_init(&rb, capacity, malloc, struct_size); + + int data = 10; + + rb_push_back(&rb, (void *)&data, memcpy); + + assert_int_equal(rb.capacity, capacity); + assert_int_equal(rb.count, 1); + + rb_destroy(&rb, free); + + assert_null(rb.buffer); +} + +/** Tests fill, but not overflow/wrap-around */ +static void +test_rb_push_back_fill(void **state) { + struct RingBuf rb = { 0, 0, 0, NULL, NULL, NULL, NULL }; + rb_size_t capacity = 10; + rb_size_t struct_size = sizeof(int); + + rb_init(&rb, capacity, malloc, struct_size); + + const int arr[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; + + // Insert them all sequentially + for(int i = 0; rb_push_back(&rb, &arr[i], memcpy) == WriteOk; i++); + assert_int_equal(rb.count, 10); + assert_int_equal(rb.capacity, capacity); + + // Read them out and assert expected value + for(int i = 0, d = __INT_MAX__; rb_pop_front(&rb, &d, memcpy) == ReadOk; + i++) { + assert_int_equal(d, arr[i]); + } + + assert_int_equal(rb.capacity, capacity); + assert_int_equal(rb.count, 0); + + rb_destroy(&rb, free); + + assert_null(rb.buffer); +} + +int +main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_rb_init_empty), + cmocka_unit_test(test_rb_push_back), + cmocka_unit_test(test_rb_init_destroy), + cmocka_unit_test(test_rb_push_back_fill), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +}