Codebase review fixes: CMake propagation, asserts, docs, style

.pre-commit-config.yaml

@@ -1,5 +1,5 @@
 repos:
 - repo: https://github.com/pre-commit/mirrors-clang-format
-  rev: v15.0.7
+  rev: v19.1.7
   hooks:
   - id: clang-format

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 1.3.9
+
+- Fixed `LFBB_MULTICORE_HOSTED` not being forwarded as a value through CMake and being exposed as `PRIVATE`, which silently ignored the user-supplied value and caused a struct layout mismatch between the library build and consumers
+- Tightened debug-time asserts: `LFBB_Init` now rejects single-byte buffers, `LFBB_ReadRelease` asserts the read amount stays within the buffer
+- Documentation, style and tooling fixes
+
 ## 1.3.8
 
 - Avoided potential false sharing due to wrapping indexes for performance
@@ -37,7 +43,7 @@
 
 ## 1.2.2
 
-- Improved performance by using a write_wrapped flag instead of infering write wraps
+- Improved performance by using a write_wrapped flag instead of inferring write wraps
 
 ## 1.2.1
 

CMakeLists.txt

@@ -4,20 +4,23 @@
 cmake_minimum_required(VERSION 3.16)
 
 project(lfbb
-    VERSION 1.3.8
+    VERSION 1.3.9
     LANGUAGES C CXX
 )
 
 add_subdirectory(lfbb)
 
 # Library configuration
 if (DEFINED LFBB_MULTICORE_HOSTED)
-    target_compile_definitions(${PROJECT_NAME} PRIVATE LFBB_MULTICORE_HOSTED)
+    target_compile_definitions(${PROJECT_NAME}
+    PUBLIC
+        LFBB_MULTICORE_HOSTED=${LFBB_MULTICORE_HOSTED}
+    )
 endif()
 
 if (DEFINED LFBB_CACHELINE_LENGTH)
     target_compile_definitions(${PROJECT_NAME}
-    PRIVATE
+    PUBLIC
         LFBB_CACHELINE_LENGTH=${LFBB_CACHELINE_LENGTH}
     )
 endif()

README.md

@@ -1,7 +1,7 @@
 # LFBB - Lock Free Bipartite Buffer
-![CMake](https://github.com/DNedic/lfbb/actions/workflows/.github/workflows/cmake.yml/badge.svg)
+![CMake](https://github.com/DNedic/lfbb/actions/workflows/cmake.yml/badge.svg)
 
-LFBB is a bipartite buffer implementation written in standard C11, suitable for all platforms, from deeply embedded to HPC uses. It is lock-free for single consumer single producer scenarios making it incredibly performant and easy to use.
+LFBB is a bipartite buffer implementation written in standard C11, suitable for all platforms, from deeply embedded to HPC uses. It is lock-free for single-producer, single-consumer scenarios making it incredibly performant and easy to use.
 
 ## What is a bipartite buffer
 
@@ -16,7 +16,7 @@ A bipartite buffer should be used everywhere a ring buffer is used if you want:
 
 ## Features
 * Written in standard C11, compatible with all platforms supporting it
-* Lock free thread and multicore safe in single producer single consumer scenarios
+* Lock free thread and multicore safe in single-producer, single-consumer scenarios
 * No dynamic allocation
 * Optimized for high performance
 * MIT Licensed
@@ -49,21 +49,21 @@ if (!write_started) {
         write_started = true;
     }
 } else {
-    if (ADC_PollDmaComplete(&adc_dma_h) {
+    if (ADC_PollDmaComplete(&adc_dma_h)) {
         LFBB_WriteRelease(&lfbb_adc, sizeof(data));
         write_started = false;
     }
 }
 ```
 
 ## Configuration
-The library offers two configuration defines ```LFBB_MULTICORE_HOSTED``` and ```LFBB_CACHELINE_LENGTH``` that can be passed by the build system or defined before including the library if the configuration isn't suitable.
+The library offers two configuration defines `LFBB_MULTICORE_HOSTED` and `LFBB_CACHELINE_LENGTH` that can be passed by the build system or defined before including the library if the configuration isn't suitable.
 
-On embedded systems it is usually required to do manual cache synchronization, so ```LFBB_MULTICORE_HOSTED``` should be left as ```false``` to avoid wasting space on padding for cacheline alignment of indexes.
+On embedded systems it is usually required to do manual cache synchronization, so `LFBB_MULTICORE_HOSTED` should be left as `false` to avoid wasting space on padding for cacheline alignment of indexes.
 
-For hosted systems the [False Sharing](https://en.wikipedia.org/wiki/False_sharing) phenomenom can reduce performance to some extent which is why passing ```LFBB_MULTICORE_HOSTED``` as ```true``` is advisable. This aligns the indexes to the system cacheline size, ```64``` by default.
+For hosted systems the [False Sharing](https://en.wikipedia.org/wiki/False_sharing) phenomenon can reduce performance to some extent which is why passing `LFBB_MULTICORE_HOSTED` as `true` is advisable. This aligns the indexes to the system cacheline size, `64` by default.
 
-Some systems have a non-typical cacheline length (for instance the apple M1/M2 CPUs have a cacheline length of 128 bytes), and ```LFBB_CACHELINE_LENGTH``` should be set accordingly in those cases.
+Some systems have a non-typical cacheline length (for instance the apple M1/M2 CPUs have a cacheline length of 128 bytes), and `LFBB_CACHELINE_LENGTH` should be set accordingly in those cases.
 
 ## How it works
 The Bipartite Buffer uses the same base principle as the [ring buffer data structure](https://en.wikipedia.org/wiki/Circular_buffer), however its ability to provide contiguous space for writing and reading requires modifying the approach slightly.

lfbb/inc/lfbb.h

@@ -34,7 +34,7 @@
  * This file is part of LFBB - Lock Free Bipartite Buffer
  *
  * Author:          Djordje Nedic <nedic.djordje2@gmail.com>
- * Version:         1.3.8
+ * Version:         1.3.9
  **************************************************************/
 
 /************************** INCLUDE ***************************/
@@ -90,41 +90,43 @@ typedef struct {
 
 /**
  * @brief Initializes a bipartite buffer instance
- * @param[in] Instance pointer
- * @param[in] Data array pointer
- * @param[in] Size of data array
+ * @param[in] inst Instance pointer
+ * @param[in] data_array Data array pointer
+ * @param[in] size Size of data array (must be greater than 1)
  * @retval None
  */
 void LFBB_Init(LFBB_Inst_Type *inst, uint8_t *data_array, size_t size);
 
 /**
  * @brief Acquires a linear region in the bipartite buffer for writing
- * @param[in] Instance pointer
- * @param[in] Free linear space in the buffer required
- * @retval Pointer to the beginning of the linear space
+ * @param[in] inst Instance pointer
+ * @param[in] free_required Free linear space in the buffer required
+ * @retval Pointer to the beginning of the linear space, or NULL if a
+ *         contiguous region of the requested size is not available
  */
 uint8_t *LFBB_WriteAcquire(LFBB_Inst_Type *inst, size_t free_required);
 
 /**
  * @brief Releases the bipartite buffer after a write
- * @param[in] Instance pointer
- * @param[in] Bytes written to the linear space
+ * @param[in] inst Instance pointer
+ * @param[in] written Bytes written to the linear space
  * @retval None
  */
 void LFBB_WriteRelease(LFBB_Inst_Type *inst, size_t written);
 
 /**
  * @brief Acquires a linear region in the bipartite buffer for reading
- * @param[in] Instance pointer
- * @param[out] Available linear data in the buffer
- * @retval Pointer to the beginning of the data
+ * @param[in] inst Instance pointer
+ * @param[out] available Available linear data in the buffer
+ * @retval Pointer to the beginning of the data, or NULL if the buffer is
+ *         empty (in which case \p available is set to 0)
  */
 uint8_t *LFBB_ReadAcquire(LFBB_Inst_Type *inst, size_t *available);
 
 /**
  * @brief Releases the bipartite buffer after a read
- * @param[in] Instance pointer
- * @param[in] Bytes read from the linear region
+ * @param[in] inst Instance pointer
+ * @param[in] read Bytes read from the linear region
  * @retval None
  */
 void LFBB_ReadRelease(LFBB_Inst_Type *inst, size_t read);

lfbb/src/lfbb.c

@@ -34,7 +34,7 @@
  * This file is part of LFBB - Lock Free Bipartite Buffer
  *
  * Author:          Djordje Nedic <nedic.djordje2@gmail.com>
- * Version:         1.3.8
+ * Version:         1.3.9
  **************************************************************/
 
 /************************** INCLUDE ***************************/
@@ -58,7 +58,9 @@ static size_t CalcFree(size_t w, size_t r, size_t size);
 void LFBB_Init(LFBB_Inst_Type *inst, uint8_t *data_array, const size_t size) {
     assert(inst != NULL);
     assert(data_array != NULL);
-    assert(size != 0U);
+    /* One slot is always reserved to distinguish empty from full, so the
+     * minimum useful buffer size is 2. */
+    assert(size > 1U);
 
     inst->data = data_array;
     inst->size = size;
@@ -145,7 +147,7 @@ uint8_t *LFBB_ReadAcquire(LFBB_Inst_Type *inst, size_t *available) {
 
     /* When read and write indexes are equal, the buffer is empty */
     if (r == w) {
-        *available = 0;
+        *available = 0U;
         return NULL;
     }
 
@@ -182,6 +184,7 @@ void LFBB_ReadRelease(LFBB_Inst_Type *inst, const size_t read) {
     }
 
     /* Increment the read index and wrap to 0 if needed */
+    assert(r + read <= inst->size);
     r += read;
     if (r == inst->size) {
         r = 0U;

tests/tests.cpp

@@ -62,7 +62,7 @@ TEST_CASE("Write with overflow condition", "[write_overflow]") {
     uint8_t *read_location = LFBB_ReadAcquire(&lfbb, &read_available);
     LFBB_ReadRelease(&lfbb, sizeof(test_data));
 
-    /* Write again, this time the oveflow will trigger and should give us the
+    /* Write again, this time the overflow will trigger and should give us the
      * beginning again */
     const uint8_t test_data2[240] = {0xA3U};
     write_location = LFBB_WriteAcquire(&lfbb, sizeof(test_data2));
@@ -93,7 +93,7 @@ TEST_CASE("Read data written after overflow condition write",
     uint8_t *read_location = LFBB_ReadAcquire(&lfbb, &read_available);
     LFBB_ReadRelease(&lfbb, sizeof(test_data));
 
-    /* Write again, this time the oveflow will trigger and should give us the
+    /* Write again, this time the overflow will trigger and should give us the
      * beginning again */
     const uint8_t test_data2[240] = {0xA3U};
     write_location = LFBB_WriteAcquire(&lfbb, sizeof(test_data2));