Merge pull request 'refactor-consistency' (#1) from refactor-consistency into main

Reviewed-on: #1

CMakeLists.txt

@@ -1,39 +1,34 @@
 cmake_minimum_required(VERSION 3.20)
-project(calculator VERSION 1.0 LANGUAGES C)
+project(arena VERSION 1.0 LANGUAGES C)
 
 set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_STANDARD_REQUIRED ON)
 
-# Export compile_commands.json (para clangd)
+# Opciones
-set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+option(ARENA_BUILD_TESTS "Build arena tests" OFF)
+option(ARENA_ENABLE_SANITIZERS "Enable sanitizers for tests" ON)
 
-add_compile_options(
+# Librería
+add_library(arena
+    src/arena.c
+)
+
+target_include_directories(arena
+    PUBLIC
+        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
+        $<INSTALL_INTERFACE:include>
+)
+
+target_compile_options(arena PRIVATE
     -Wall
     -Wextra
     -Wpedantic
 )
 
-
-include_directories(include)
-
-add_library(arena_lib
-    src/arena.c
-)
-
-add_executable(arena_main src/main.c)
-
-target_link_libraries(arena_main arena_lib)
-
-function(enable_sanitizers target)
-    target_compile_options(${target} PRIVATE -fsanitize=address -fno-omit-frame-pointer)
-    target_link_options(${target} PRIVATE -fsanitize=address)
-endfunction()
-
-
 # ------------------------
 # Testing
 # ------------------------
-
+if(ARENA_BUILD_TESTS)
     enable_testing()
     add_subdirectory(test)
-
+endif()

include/arena.h

@@ -6,11 +6,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 
-typedef struct {
+typedef struct Arena Arena;
-    uint8_t *buffer;
-    size_t capacity;
-    size_t offset;
-} Arena;
 
 typedef enum {
     ARENA_OK = 0,
@@ -26,7 +22,10 @@ typedef struct {
     bool is_valid;
     union {
         ArenaErr err;
-        uint8_t *address;
+        struct {
+            Arena *arena;
+            size_t offset;
+        };
     };
 } ArenaPointer;
 
@@ -34,7 +33,7 @@ typedef struct {
     bool is_valid;
     union {
         ArenaErr err;
-        Arena arena;
+        Arena *arena;
     };
 } ArenaResult;
 
@@ -44,23 +43,21 @@ typedef struct {
         ArenaErr err;
         size_t val;
     };
-} SizeResult;
+} ArenaSizeResult;
 
-ArenaResult arena_init(size_t capacity);
+ArenaErr arena_init(Arena **arena, size_t capacity);
-void arena_destroy(Arena *arena);
+ArenaErr arena_destroy(Arena **arena);
 
 ArenaPointer arena_alloc(Arena *arena, size_t size, size_t alignment);
 ArenaPointer arena_push(Arena *arena, void *data, size_t size, size_t alignment);
 ArenaErr arena_realloc(Arena *arena, size_t new_capacity);
+void *arena_unwrap_pointer(ArenaPointer p);
 
-SizeResult get_arena_align_padding(Arena *arena, size_t alignment);
+ArenaSizeResult arena_get_align_padding(Arena *arena, size_t alignment);
+// can take big sizes but of sourse there is a limit,
+// its protected against SIZE_MAX but anything that
+// is big may break it, still, is an edge case that
+// I shouldn't cover.
 ArenaErr arena_ensure_capacity(Arena *arena, size_t size, size_t alignment);
-// Should be moved to something like general utilities,
-// i should make one for all my c projects
-bool mul_size_t_safe(size_t a, size_t b, size_t *out);
-
-
-
-
 
 #endif // !ARENA_H

src/arena.c

@@ -5,30 +5,56 @@
 #include <stdlib.h>
 #include <string.h>
 
-ArenaResult arena_init(size_t capacity) {
+struct Arena {
+    uint8_t *buffer;
+    size_t capacity;
+    size_t offset;
+};
+
+
+ArenaErr arena_init(Arena **arena, size_t capacity) {
+    if (arena == NULL) {
+        return ARENA_NULL_ARG;
+    }
+
     if (capacity < 1) {
-        return (ArenaResult) {.is_valid = false, .err = ARENA_INVALID_SIZE};
+        return ARENA_INVALID_SIZE;
+    }
+
+    Arena *new_arena = malloc(sizeof(Arena));
+    if (new_arena == NULL) {
+        return ARENA_BAD_ALLOC;
     }
 
     void *buffer = malloc(capacity);
     if (buffer == NULL) {
-        return (ArenaResult) {.is_valid = false, .err = ARENA_BAD_ALLOC};
+        return ARENA_BAD_ALLOC;
     }
 
-    Arena new_arena = {
+    *new_arena = (Arena) {
         .buffer = buffer,
         .capacity = capacity,
         .offset = 0,
     };
 
-    return (ArenaResult) {.is_valid = true, .arena = new_arena};
+    *arena = new_arena;
+
+    return ARENA_OK;
 }
 
-void arena_destroy(Arena *arena) {
+ArenaErr arena_destroy(Arena **arena) {
-    free(arena->buffer);
+    if (arena == NULL || *arena == NULL) {
-    arena->buffer = NULL;
+        return ARENA_NULL_ARG;
-    arena->offset = 0;
+    }
-    arena->capacity = 0;
+
+    free((*arena)->buffer);
+    (*arena)->buffer = NULL;
+    (*arena)->offset = 0;
+    (*arena)->capacity = 0;
+    free(*arena);
+    *arena = NULL;
+
+    return ARENA_OK;
 }
 
 ArenaPointer arena_alloc(Arena *arena, size_t size, size_t alignment) {
@@ -36,13 +62,25 @@ ArenaPointer arena_alloc(Arena *arena, size_t size, size_t alignment) {
         return (ArenaPointer) {.is_valid = false, .err = ARENA_NULL_ARG};
     }
 
-    SizeResult padding = get_arena_align_padding(arena, alignment);
+    if (size < 1) {
+        return (ArenaPointer) {.is_valid = false, .err = ARENA_INVALID_SIZE};
+    }
+
+    if (alignment < 1) {
+        return (ArenaPointer) {.is_valid = false, .err = ARENA_INVALID_ALIGN};
+    }
+
+    ArenaSizeResult padding = arena_get_align_padding(arena, alignment);
     if (!padding.is_valid) {
         return (ArenaPointer) {.is_valid = false, .err = padding.err};
     }
 
+    ArenaErr cap_err = arena_ensure_capacity(arena, size, alignment);
+    if(cap_err != ARENA_OK) {
+        return (ArenaPointer) {.is_valid = false, .err = ARENA_OUT_OF_SPACE};
+    }
 
-    if (arena->offset + padding.val >= arena->capacity) {
+    if (arena->offset + padding.val + size > arena->capacity) {
         return (ArenaPointer) {.is_valid = false, .err = ARENA_OUT_OF_SPACE};
     }
 
@@ -53,7 +91,7 @@ ArenaPointer arena_alloc(Arena *arena, size_t size, size_t alignment) {
     size_t aligned_offset = arena->offset + padding.val;
 
     arena->offset = aligned_offset + size;
-    return (ArenaPointer) {.is_valid = true, .address = arena->buffer + aligned_offset};
+    return (ArenaPointer) {.is_valid = true, .offset = aligned_offset, .arena = arena};
 }
 
 ArenaErr arena_ensure_capacity(Arena *arena, size_t size, size_t alignment) {
@@ -61,26 +99,50 @@ ArenaErr arena_ensure_capacity(Arena *arena, size_t size, size_t alignment) {
         return ARENA_NULL_ARG;
     }
 
-    SizeResult padding = get_arena_align_padding(arena, alignment);
+    if (size == 0 || size == SIZE_MAX) {
-    if (!padding.is_valid) {
+        return ARENA_INVALID_SIZE;
-        return padding.err;
-    }
-    while (true) {
-        if (arena->offset > SIZE_MAX - padding.val - size) {
-            return ARENA_CAPACITY_OVERFLOW;
     }
 
-        size_t required = arena->offset + padding.val + size;
+    if (alignment == 0 || (alignment & (alignment - 1)) != 0) {
+        return ARENA_INVALID_ALIGN;
+    }
+
+    ArenaSizeResult padding_res = arena_get_align_padding(arena, alignment);
+    if (!padding_res.is_valid) {
+        return padding_res.err;
+    }
+
+    size_t padding = padding_res.val;
+
+    while (true) {
+        size_t required;
+
+        if (arena->offset > SIZE_MAX - padding) {
+            return ARENA_CAPACITY_OVERFLOW;
+        }
+        required = arena->offset + padding;
+
+        if (required > SIZE_MAX - size) {
+            return ARENA_CAPACITY_OVERFLOW;
+        }
+        required += size;
 
         if (required <= arena->capacity) {
             return ARENA_OK;
         }
 
         size_t new_capacity;
-        if (mul_size_t_safe(arena->capacity, 2, &new_capacity)) {
+
+        if (arena->capacity > SIZE_MAX / 2) {
             return ARENA_CAPACITY_OVERFLOW;
         }
 
+        new_capacity = arena->capacity * 2;
+
+        if (new_capacity < required) {
+            new_capacity = required;
+        }
+
         ArenaErr err = arena_realloc(arena, new_capacity);
         if (err != ARENA_OK) {
             return err;
@@ -93,6 +155,14 @@ ArenaPointer arena_push(Arena *arena, void *data, size_t size, size_t alignment)
         return (ArenaPointer) {.is_valid = false, .err = ARENA_NULL_ARG};
     }
 
+    if (size < 1) {
+        return (ArenaPointer) {.is_valid = false, .err = ARENA_INVALID_SIZE};
+    }
+
+    if (size < 1) {
+        return (ArenaPointer) {.is_valid = false, .err = ARENA_INVALID_ALIGN};
+    }
+
     ArenaErr err = arena_ensure_capacity(arena, size, alignment);
     if (err != ARENA_OK) {
         return (ArenaPointer) {.is_valid = false, .err = err};
@@ -104,7 +174,7 @@ ArenaPointer arena_push(Arena *arena, void *data, size_t size, size_t alignment)
     }
     
     memcpy(
-        pointer.address,
+        arena_unwrap_pointer(pointer),
         data,
         size);
 
@@ -116,6 +186,10 @@ ArenaErr arena_realloc(Arena *arena, size_t new_capacity) {
         return ARENA_NULL_ARG;
     }
 
+    if (new_capacity < 1) {
+        return ARENA_INVALID_SIZE;
+    }
+
     uint8_t *tmp = realloc(arena->buffer, new_capacity);
     if (tmp == NULL) {
         return ARENA_BAD_ALLOC;
@@ -126,31 +200,33 @@ ArenaErr arena_realloc(Arena *arena, size_t new_capacity) {
     return ARENA_OK;
 }
 
-SizeResult get_arena_align_padding(Arena *arena, size_t alignment) {
+
+
+ArenaSizeResult arena_get_align_padding(Arena *arena, size_t alignment) {
     if (arena == NULL) {
-        return (SizeResult) {.is_valid = false, .err = ARENA_NULL_ARG};
+        return (ArenaSizeResult){ .is_valid = false, .err = ARENA_NULL_ARG };
     }
 
-    if (alignment < 1) {
+    if (alignment == 0 || (alignment & (alignment - 1)) != 0) {
-        return (SizeResult) {.is_valid = false, .err = ARENA_INVALID_ALIGN};
+        return (ArenaSizeResult){ .is_valid = false, .err = ARENA_INVALID_ALIGN };
     }
 
     uintptr_t current_address = (uintptr_t)(arena->buffer + arena->offset);
-    uintptr_t aligned = ((current_address + (alignment - 1)) & ~(alignment - 1));
 
-    return (SizeResult) {.is_valid = true, .err = aligned - current_address};
+    size_t mask = alignment - 1;
+    size_t padding = (size_t)(-current_address) & mask;
+
+    return (ArenaSizeResult){
+        .is_valid = true,
+        .val = padding
+    };
 }
 
-bool mul_size_t_safe(size_t a, size_t b, size_t *out) {
+void *arena_unwrap_pointer(ArenaPointer p) {
-    if (out == NULL) {
+    if (!p.is_valid) {
-        return true;
+        return NULL;
+    }
+    return p.arena->buffer + p.offset;
 }
 
-    if (a != 0 && b > SIZE_MAX / a) {
-        return true;
-    }
-
-    *out = a * b;
-    return false;
-}
 

test/CMakeLists.txt

@@ -3,13 +3,21 @@ find_package(cmocka REQUIRED)
 add_executable(test_arena test_arena.c)
 
 target_link_libraries(test_arena
-    arena_lib
+    arena
     cmocka::cmocka
 )
 
-enable_sanitizers(test_arena)
+# Sanitizers (solo si están activados y el compilador lo soporta)
-
+if(ARENA_ENABLE_SANITIZERS)
-target_compile_options(test_arena PRIVATE -fsanitize=address -fno-omit-frame-pointer)
+    if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang")
-target_link_options(test_arena PRIVATE -fsanitize=address)
+        target_compile_options(test_arena PRIVATE
+            -fsanitize=address
+            -fno-omit-frame-pointer
+        )
+        target_link_options(test_arena PRIVATE
+            -fsanitize=address
+        )
+    endif()
+endif()
 
 add_test(NAME arena_tests COMMAND test_arena)

test/test_arena.c

@@ -7,30 +7,31 @@
 #include <setjmp.h>
 #include <cmocka.h>
 #include <string.h>
+#include <limits.h>
 
 #include "arena.h"
 
 static void test_push_3_ints(void **state) {
     (void) state;
 
-    ArenaResult value = arena_init(sizeof(int) * 3);
+    Arena *arena;
-    assert_true(value.is_valid);
+    assert_uint_equal(arena_init(&arena, sizeof(int) * 3), ARENA_OK);
-    Arena arena = value.arena;
 
     int int_to_push = 20;
-    ArenaPointer result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    ArenaPointer result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(20, *(int*)result.address);
+    assert_int_equal(20, *(int*)arena_unwrap_pointer(result));
+    assert_int_equal(20, *(int*)arena_unwrap_pointer(result));
 
     int_to_push = 30;
-    result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(30, *(int*) result.address);
+    assert_int_equal(30, *(int*)arena_unwrap_pointer(result));
 
     int_to_push = 40;
-    result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(40, *(int*) result.address);
+    assert_int_equal(40, *(int*)arena_unwrap_pointer(result));
 
     arena_destroy(&arena);
 }
@@ -38,43 +39,174 @@ static void test_push_3_ints(void **state) {
 static void test_push_3_ints_2_doubles(void **state) {
     (void) state;
 
-    ArenaResult value = arena_init((sizeof(int) * 3) + (sizeof(double) * 2));
+    Arena *arena;
-    assert_true(value.is_valid);
+    assert_uint_equal(
-    Arena arena = value.arena;
+        arena_init(&arena, (sizeof(int) * 3) + (sizeof(double) * 2)),
+        ARENA_OK
+    );
 
     int int_to_push = 20;
-    ArenaPointer result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    ArenaPointer result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(20, *(int*)result.address);
+    assert_int_equal(20, *(int*)arena_unwrap_pointer(result));
 
     double double_to_push = 4.57;
-    result = arena_push(&arena, &double_to_push, sizeof(double), alignof(double));
+    result = arena_push(arena, &double_to_push, sizeof(double), alignof(double));
     assert_true(result.is_valid);
-    assert_double_equal(4.57, *(double*)result.address, 1e-6);
+    assert_double_equal(4.57, *(double*)arena_unwrap_pointer(result), 1e-6);
 
     int_to_push = 30;
-    result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(30, *(int*)result.address);
+    assert_int_equal(30, *(int*)arena_unwrap_pointer(result));
 
     int_to_push = 40;
-    result = arena_push(&arena, &int_to_push, sizeof(int), alignof(int));
+    result = arena_push(arena, &int_to_push, sizeof(int), alignof(int));
     assert_true(result.is_valid);
-    assert_int_equal(40, *(int*) result.address);
+    assert_int_equal(40, *(int*)arena_unwrap_pointer(result));
 
     double_to_push = 267.33;
-    result = arena_push(&arena, &double_to_push, sizeof(double), alignof(double));
+    result = arena_push(arena, &double_to_push, sizeof(double), alignof(double));
     assert_true(result.is_valid);
-    assert_double_equal(267.33, *(double*) result.address, 1e-6);
+    assert_double_equal(267.33, *(double*)arena_unwrap_pointer(result), 1e-6);
 
     arena_destroy(&arena);
 }
 
+static void test_init_arena_0_cap(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_int_equal(arena_init(&arena, 0), ARENA_INVALID_SIZE);
+}
+
+static void test_arena_alloc_size_0(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaPointer pointer = arena_alloc(arena, 0, alignof(int));
+    assert_false(pointer.is_valid);
+    assert_int_equal(pointer.err, ARENA_INVALID_SIZE);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_alloc_size_max(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaPointer pointer = arena_alloc(arena, SIZE_MAX, alignof(int));
+    assert_false(pointer.is_valid);
+    assert_int_equal(pointer.err, ARENA_OUT_OF_SPACE);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_alloc_align_0(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaPointer pointer = arena_alloc(arena, sizeof(int), 0);
+    assert_false(pointer.is_valid);
+    assert_int_equal(pointer.err, ARENA_INVALID_ALIGN);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_alloc_align_max(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaPointer pointer = arena_alloc(arena, sizeof(int), SIZE_MAX);
+    assert_false(pointer.is_valid);
+    assert_int_equal(pointer.err, ARENA_INVALID_ALIGN);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_align_size_0(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaPointer pointer = arena_alloc(arena, sizeof(int), 0);
+    assert_false(pointer.is_valid);
+    assert_int_equal(pointer.err, ARENA_INVALID_ALIGN);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_ensure_capacity_size_0(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaErr err = arena_ensure_capacity(arena, 0, alignof(int));
+    assert_int_equal(err, ARENA_INVALID_SIZE);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_ensure_capacity_size_max(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaErr err = arena_ensure_capacity(arena, SIZE_MAX, alignof(int));
+    assert_int_equal(err, ARENA_INVALID_SIZE);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_ensure_capacity_align_0(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaErr err = arena_ensure_capacity(arena, sizeof(int), 0);
+    assert_int_equal(err, ARENA_INVALID_ALIGN);
+
+    arena_destroy(&arena);
+}
+
+static void test_arena_ensure_capacity_align_max(void **state) {
+    (void) state;
+
+    Arena *arena;
+    assert_uint_equal(arena_init(&arena, 64), ARENA_OK);
+
+    ArenaErr err = arena_ensure_capacity(arena, sizeof(int), SIZE_MAX);
+    assert_int_equal(err, ARENA_INVALID_ALIGN);
+
+    arena_destroy(&arena);
+}
 
 int main(void) {
     const struct CMUnitTest tests[] = {
         cmocka_unit_test(test_push_3_ints),
         cmocka_unit_test(test_push_3_ints_2_doubles),
+        cmocka_unit_test(test_init_arena_0_cap),
+        cmocka_unit_test(test_arena_alloc_size_0),
+        cmocka_unit_test(test_arena_alloc_size_max),
+        cmocka_unit_test(test_arena_alloc_align_0),
+        cmocka_unit_test(test_arena_alloc_align_max),
+        cmocka_unit_test(test_arena_align_size_0),
+        cmocka_unit_test(test_arena_ensure_capacity_align_0),
+        cmocka_unit_test(test_arena_ensure_capacity_align_max),
+        cmocka_unit_test(test_arena_ensure_capacity_size_0),
+        cmocka_unit_test(test_arena_ensure_capacity_size_max),
     };