diff --git a/CMakeLists.txt b/CMakeLists.txt index 39285f2..2f1ee7e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,9 @@ cmake_minimum_required(VERSION 3.10) -project(akerror LANGUAGES C) +# The status-code registry replaced the consumer-sized __AKERR_ERROR_NAMES array +# with private storage, which is a source and ABI break for anything built +# against an earlier header -- hence 1.0.0 and a SOVERSION, so a stale installed +# libakerror.so can no longer be silently paired with new headers. +project(akerror VERSION 1.0.0 LANGUAGES C) include(GNUInstallDirs) include(CMakePackageConfigHelpers) @@ -7,6 +11,18 @@ include(CTest) set(AKERR_USE_STDLIB 1 CACHE BOOL "Use the C standard library") set(AKERR_COVERAGE 0 CACHE BOOL "Instrument the build with gcov coverage counters") + +# Size of the private status-name hash table. Must be a power of two; usable +# capacity is 75% of it (src/error.c asserts both). The host's errno list +# consumes part of that at akerr_init() time, so the remainder is what all +# consumer libraries in the process share. These are applied PRIVATE on purpose: +# the table lives entirely in src/error.c, so raising them never changes +# anything a consumer can see. That is what makes them safe to tune, unlike the +# AKERR_MAX_ERR_VALUE they replaced. +set(AKERR_STATUS_NAME_SLOTS 4096 CACHE STRING + "Slots in the status-name table (power of two; 75% usable)") +set(AKERR_MAX_RESERVED_STATUS_RANGES 64 CACHE STRING + "Maximum number of status ranges that may be reserved") set(akerror_install_cmakedir "${CMAKE_INSTALL_LIBDIR}/cmake/akerror") # Coverage instrumentation. Applied per target (not globally) so it never leaks @@ -77,6 +93,13 @@ add_library(akerror::akerror ALIAS akerror) target_compile_definitions(akerror PUBLIC AKERR_USE_STDLIB=${AKERR_USE_STDLIB} + PRIVATE AKERR_STATUS_NAME_SLOTS=${AKERR_STATUS_NAME_SLOTS} + PRIVATE AKERR_MAX_RESERVED_STATUS_RANGES=${AKERR_MAX_RESERVED_STATUS_RANGES} +) + +set_target_properties(akerror PROPERTIES + VERSION ${PROJECT_VERSION} + SOVERSION ${PROJECT_VERSION_MAJOR} ) akerr_instrument_for_coverage(akerror) @@ -104,6 +127,8 @@ set(AKERR_TESTS err_release_clears err_pool_exhaust err_maxval + err_name_ownership + err_registry_init_order err_refcount_double_fail err_stacktrace_bounds err_name_bounds diff --git a/README.md b/README.md index 5dc8525..0d2eab2 100644 --- a/README.md +++ b/README.md @@ -4,49 +4,154 @@ This library provides a TRY/CATCH style exception handling mechanism for C. ![build badge](https://source.starfort.tech/andrew/libakerror/actions/workflows/ci.yaml/badge.svg?branch=main) -## Upgrade notice: custom status codes +## Upgrade notice: custom status codes (1.0.0) -This version changes how custom status-code names and ownership are managed. Applications and libraries upgrading from an earlier version should review every custom error-code definition and initialization path. +Version 1.0.0 replaces the consumer-sized status-name array with a private +registry, and makes status-code ownership explicit and enforced. This is a +source and ABI break. The library now carries a version and an soname +(`libakerror.so.1`), so a stale installed library can no longer be silently +paired with a newer header — but anything built against a pre-1.0.0 header must +be rebuilt. -To comply with the new status-code rules: +What was removed: -1. Rebuild libakerror and all dependent libraries against the new header. -2. Remove `AKERR_MAX_ERR_VALUE` compile definitions and source references. The status-name registry is now sparse and accepts any `int`; consumers no longer configure its size. -3. Move custom status codes out of the reserved `0` through `255` range. Use fixed integer constants beginning at `256`, rather than offsets from `AKERR_LAST_ERRNO_VALUE`, so libc changes cannot move your codes. -4. Assign a distinct range to every library that may coexist in one process. Coordinate these ranges at the application or dependency-stack level. -5. After `akerr_init()` and before raising or naming custom errors, reserve the complete range with `akerr_reserve_status_range()`. Treat every result other than `AKERR_STATUS_RANGE_OK` as an initialization failure. -6. Register human-readable names with `akerr_name_for_status()` as before. Every co-resident library must reserve its range for collision detection to be effective. +* `AKERR_MAX_ERR_VALUE` — the registry is sparse and accepts any `int`, so + consumers no longer size it. Delete every compile definition and source + reference. A stale `-DAKERR_MAX_ERR_VALUE=...` is now harmless but useless. +* `__AKERR_ERROR_NAMES` — the name table is private to the library. Code that + touched this data symbol was using an undocumented interface; use + `akerr_name_for_status()`. + +To migrate: + +1. Rebuild libakerror and every dependent library against the new header. +2. Move custom status codes out of the reserved `0`–`255` band. Use fixed + integer constants beginning at `AKERR_FIRST_CONSUMER_STATUS` (256) rather + than offsets from `AKERR_LAST_ERRNO_VALUE`, so a libc that grows an errno + cannot move your codes. +3. Assign a distinct range to every library that may coexist in one process, and + coordinate those ranges at the application or dependency-stack level. +4. Reserve the complete range with `akerr_reserve_status_range()` during + initialization. Treat any result other than `AKERR_STATUS_RANGE_OK` as an + initialization failure. +5. Register names with `akerr_register_status_name()`, passing the same owner + string you reserved with. **You can no longer name a status you have not + reserved** — see "Ownership is enforced" below. For example: ```c enum { - MYLIB_ERR_BASE = 256, + MYLIB_ERR_BASE = AKERR_FIRST_CONSUMER_STATUS, /* 256 */ MYLIB_ERR_PARSE = MYLIB_ERR_BASE, MYLIB_ERR_STORAGE, MYLIB_ERR_LIMIT }; +#define MYLIB_OWNER "mylib" + int mylib_init(void) { - int result; - - akerr_init(); - result = akerr_reserve_status_range( - MYLIB_ERR_BASE, - MYLIB_ERR_LIMIT - MYLIB_ERR_BASE, - "mylib"); - if (result != AKERR_STATUS_RANGE_OK) { - return result; + if (akerr_reserve_status_range(MYLIB_ERR_BASE, + MYLIB_ERR_LIMIT - MYLIB_ERR_BASE, + MYLIB_OWNER) != AKERR_STATUS_RANGE_OK) { + return MYLIB_INIT_FAILED; /* another component owns part of the range */ } - akerr_name_for_status(MYLIB_ERR_PARSE, "Parse Error"); - akerr_name_for_status(MYLIB_ERR_STORAGE, "Storage Error"); - return AKERR_STATUS_RANGE_OK; + akerr_register_status_name(MYLIB_OWNER, MYLIB_ERR_PARSE, "Parse Error"); + akerr_register_status_name(MYLIB_OWNER, MYLIB_ERR_STORAGE, "Storage Error"); + return MYLIB_INIT_OK; } ``` -Repeating the exact same reservation for the same owner is safe. Overlapping ranges, invalid ranges, and exhausted reservation capacity are rejected. Code that directly referenced the former `__AKERR_ERROR_NAMES` data symbol used an undocumented interface and must migrate to `akerr_name_for_status()`. +You do not need to call `akerr_init()` first. Every registry entry point calls +it for you, so a library that reserves its range before anything else in the +process has touched libakerror keeps that reservation. (Before 1.0.0 this was +silently destructive: `akerr_init()` cleared the tables, so a reservation made +too early was discarded and the next component to claim the same range was told +it was free.) + +### Ownership is enforced + +Reserving a range is no longer advisory bookkeeping. A status may only be named +from inside a reservation: + +* `akerr_register_status_name(owner, status, name)` requires that `status` fall + in a range reserved by `owner`. Naming another component's status fails with + `AKERR_STATUS_NAME_FOREIGN`, and naming a status nobody reserved fails with + `AKERR_STATUS_NAME_UNRESERVED`. +* The two-argument `akerr_name_for_status(status, name)` set path still works, + but it cannot identify its caller, so it can only require that *some* + reservation covers the status. Prefer the owned form: it is the one that + catches a component writing into a range that is not its own. + +Every refusal is reported through `akerr_log_method` and names the real owner, +because a name that fails to register degrades that code to `"Unknown Error"` in +every later stack trace. Registration failures are not fatal by themselves — +check the return value during initialization if you want them to be. + +This detects *name* collisions. It cannot detect two components compiling the +same integer into a `HANDLE` label without ever registering a name, so every +co-resident library should still reserve its range. + +### Return codes + +`akerr_reserve_status_range()`: + +| Code | Meaning | +| --- | --- | +| `AKERR_STATUS_RANGE_OK` (0) | Range reserved, or an identical range was already reserved by the same owner | +| `AKERR_STATUS_RANGE_OVERLAP` | Part of the range is owned by someone else (logged, with the owner) | +| `AKERR_STATUS_RANGE_FULL` | No reservation slots remain | +| `AKERR_STATUS_RANGE_INVALID` | `count < 1`, NULL/empty/over-long owner, or the range overflows `int` | + +`akerr_register_status_name()`: + +| Code | Meaning | +| --- | --- | +| `AKERR_STATUS_NAME_OK` (0) | Name registered | +| `AKERR_STATUS_NAME_UNRESERVED` | No reservation contains this status | +| `AKERR_STATUS_NAME_FOREIGN` | The status is in a range owned by someone else | +| `AKERR_STATUS_NAME_FULL` | The name registry is full | +| `AKERR_STATUS_NAME_INVALID` | NULL/empty/over-long owner, or a NULL name | + +Repeating an identical reservation for the same owner is a no-op. A *subset* or +*superset* of your own range is not — it is reported as an overlap. Reserve the +whole range in one call. + +### Capacity + +Both tables are fixed size, allocated in BSS, and never grow: + +| Limit | Default | Build-time override | +| --- | --- | --- | +| Status names | 3072 usable (4096 slots, 75% load) | `-DAKERR_STATUS_NAME_SLOTS=` | +| Reserved ranges | 64 | `-DAKERR_MAX_RESERVED_STATUS_RANGES=` | + +`akerr_init()` consumes one name slot per host errno value plus one per +`AKERR_*` code — around 150 on glibc, leaving roughly 2900 for all consumers in +the process to share. That figure varies with the host libc, so treat it as +approximate rather than a budget to fill. + +Both overrides are CMake cache variables and apply `PRIVATE` to the library +target. The tables live entirely inside `src/error.c`, so raising them changes +nothing a consumer can observe — unlike the `AKERR_MAX_ERR_VALUE` they replaced, +where a mismatch between the library and its consumers corrupted memory. Set +them when configuring libakerror itself: + +```sh +cmake -S . -B build -DAKERR_STATUS_NAME_SLOTS=16384 +``` + +Exhausting either table is reported through `akerr_log_method` and returned to +the caller; it is never silent. + +### Thread safety + +The registry is process-global mutable state with no locking. Reserve ranges and +register names during single-threaded initialization, before spawning threads. +Lookups (`akerr_name_for_status(status, NULL)`) are safe to call concurrently +once registration has finished. # Why? @@ -108,24 +213,37 @@ Any function which uses the `PREPARE_ERROR` macro should have a return type of ` The library uses integer values to specify error codes inside of its context. These integer return codes are defined in `akerror.h` in the form of `AKERR_xxxxx` where `xxxxx` is the name of the error code in question. See `akerror.h` for a list of defined errors and their descriptions. -You can define additional error types as integer constants. Values 0 through 255 are reserved by libakerror; consumers should allocate codes starting at 256. Status names are stored sparsely, so there is no maximum status value and no compiler definition is needed. +You can define additional error types as integer constants. Values 0 through 255 +are reserved by libakerror (the host's errno values plus the `AKERR_*` codes); +consumers allocate codes starting at `AKERR_FIRST_CONSUMER_STATUS` (256). Status +names are stored sparsely, so any `int` is a legal status and no compile +definition is needed to use large values. -Libraries that may coexist in one process should reserve their ranges during initialization and treat any nonzero result as a startup error: +Every library that may coexist in one process must reserve its range during +initialization and treat any nonzero result as a startup error: ```c -if (akerr_reserve_status_range(256, 16, "my-library") != AKERR_STATUS_RANGE_OK) { +#define MYLIB_OWNER "my-library" + +if (akerr_reserve_status_range(256, 16, MYLIB_OWNER) != AKERR_STATUS_RANGE_OK) { /* Refuse to initialize: another component owns part of the range. */ } ``` -Reservations are process-local, fixed-capacity, and idempotent when the same owner repeats the exact same range. They detect declared range collisions while preserving compile-time integer constants for `HANDLE`. Call `akerr_init()` before reserving ranges. Existing custom codes below 256 should migrate into a declared range at or above 256. +Reservations are process-local, fixed-capacity, and idempotent when the same +owner repeats the exact same range. They preserve compile-time integer constants +so `HANDLE` still works, since `case` labels require them. -Define a human-friendly name for the error with `akerr_name_for_status` during initialization before the error may be used: +Then name each code, quoting the owner you reserved with: ```c -akerr_name_for_status(256, "Some Error Code Description") +akerr_register_status_name(MYLIB_OWNER, 256, "Some Error Code Description"); ``` +Naming a status outside your reservation is refused and logged. See +[the upgrade notice](#upgrade-notice-custom-status-codes-100) for the return +codes, the capacity limits and how to raise them, and thread-safety rules. + # Installation @@ -149,10 +267,14 @@ This library depends upon `stdlib`. If you don't want to link against stdlib, yo - `memset` function - `strncpy` function +- `strlen` function +- `strcmp` function - `sprintf` function - `exit` function - `bool` type - `NULL` type +- `INT_MAX` constant +- `PATH_MAX` constant ... then you can compile it thusly: diff --git a/TODO.md b/TODO.md index 5fca80d..fecc5d4 100644 --- a/TODO.md +++ b/TODO.md @@ -2,10 +2,110 @@ The status-code ownership work has been consumed. The implementation now: -- stores status names in a private, fixed-capacity sparse registry, removing the consumer-visible array ABI and `AKERR_MAX_ERR_VALUE`; -- accepts names for arbitrary `int` status values and always terminates truncated names; -- reserves status values 0 through 255 for libakerror; -- provides `akerr_reserve_status_range()` so independently developed libraries can declare ranges and detect overlaps at initialization; and -- tests arbitrary status values, range overlap/idempotency/overflow, owner and name limits, and both registry capacity limits. +- stores status names in a private, open-addressed sparse registry, removing the + consumer-visible array ABI and `AKERR_MAX_ERR_VALUE`; +- accepts names for arbitrary `int` status values and always terminates + truncated names; +- reserves status values 0 through 255 for libakerror, and fails the build + (`akerr_assert_codes_within_reserved_band`) if a host errno space ever pushes + an `AKERR_*` code out of that band; +- provides `akerr_reserve_status_range()` so independently developed libraries + can declare ranges and detect overlaps at initialization; +- **enforces** those reservations: `akerr_register_status_name()` refuses to + name a status outside a range the caller owns, and the legacy two-argument + `akerr_name_for_status()` set path refuses a status nobody reserved. Every + refusal, and both capacity limits, are reported through `akerr_log_method`; +- self-initializes from every registry entry point, so a reservation made before + the first `PREPARE_ERROR` is no longer wiped by `akerr_init()`; +- names every `AKERR_*` code (`AKERR_EOF`, `AKERR_ITERATOR_BREAK` and + `AKERR_NOT_IMPLEMENTED` previously had none and rendered as "Unknown Error"); +- carries a version and soname (1.0.0 / `libakerror.so.1`) so a stale installed + library cannot be silently paired with newer headers; and +- sizes the name registry at 4096 slots (3072 usable, ~2900 free to consumers + after errno registration), tunable at build time via + `AKERR_STATUS_NAME_SLOTS` and `AKERR_MAX_RESERVED_STATUS_RANGES`. Both are + private to `src/error.c`, so raising them cannot desynchronize a library from + its consumers the way `AKERR_MAX_ERR_VALUE` could. -Consumers with existing custom codes below 256 should migrate them to a declared range starting at 256 or higher. Range reservation is opt-in, so every co-resident library must participate for collisions to be detected. +None of this introduces dynamic allocation: both tables are file-scope arrays in +BSS, and the library's undefined-symbol set remains `exit`, `memset`, `snprintf`, +`vfprintf`, `strncpy`, `strcmp`, `strlen` and `stderr` — no allocator. The name +table grew from ~34 KB to 288 KB, against the ~4.55 MB `AKERR_ARRAY_ERROR` +already occupies, so total BSS moved ~4.6 MB → ~4.9 MB. Raising +`AKERR_STATUS_NAME_SLOTS` costs 72 bytes per slot, statically. (`strcmp` and +`strlen` are new external dependencies; the README's no-stdlib list records +them.) + +Consumers with existing custom codes below 256 must migrate them to a declared +range starting at 256 or higher; `libakgl`'s codes at `AKERR_LAST_ERRNO_VALUE + +18..22` land inside the reserved band and will fail to reserve. + +## Mutation testing surfaced a latent out-of-bounds write in the new registry + +Worth recording because the failure mode is silent and the class will recur if +the registry is touched again. + +`akerr_status_slot()` advances its probe with +`slot = (slot + 1u) & (AKERR_STATUS_NAME_SLOTS - 1)`. Mutating that mask to +`- 0` or `+ 1` makes it index *past* the end of `akerr_status_names` — a write +into adjacent BSS. Both mutants **survived the test suite**: `err_maxval.c` only +asserted that *some* names registered before the table reported full, and a +probe sequence that collapses to two slots still satisfies that. The test now +requires a substantial entry count and reads every entry back by its own +distinct name, so a probe that revisits slots fails on both counts. Both mutants +are killed (`src/error.c` score 74% → 77.3%). + +Two things to keep in mind: + +- **That mask is load-bearing and fails silently.** It corrupts neighbouring + statics rather than crashing, which is exactly why a passing test suite proved + nothing. Any future change to the probe sequence, the occupancy cap, or the + power-of-two assumption needs a test that reads back *every* entry, not a + count. +- **The suite has no sanitizer run.** ASan would have caught this class directly + and independently of how sharp the assertions are, which is the more general + defense. Adding a `-fsanitize=address,undefined` build to `.gitea/workflows/ + ci.yaml` (or a CMake option alongside `AKERR_COVERAGE`) is cheap and would + cover the whole library, not just the registry. This is the highest-value + follow-up in this file. + +## Known gaps + +Ordered by how likely they are to matter as more components adopt the library. + +1. **`HANDLE`-level aliasing is still undetectable.** Two components can compile + the same integer into a `case` label without ever reserving a range or + registering a name, and nothing sees it. Enforcement covers *naming*, which + is the part the library mediates; the `case` label never reaches it. Closing + this needs the `if`/`else if` handler ladder described in option (c) of the + original notes, which is a much larger change to the macro layer. + +2. **No registry introspection.** There is no way to ask who owns a status or to + enumerate reservations, so the "coordinate ranges at the dependency-stack + level" advice has no tooling behind it. A read-only accessor plus a dump + through `akerr_log_method` would let a startup self-check or a CI job print + the whole map for a linked stack. Cheap and additive. + +3. **No way to release a reservation.** A plugin host that `dlopen`s many + distinct plugins over a process lifetime accumulates ranges until the table + fills. Reloading the *same* plugin is fine (an identical repeat is + idempotent). + +4. **The registry is not thread safe.** Global mutable state, no locking, and + `akerr_status_name_count++` is not atomic. Documented as an + initialization-time-only API; not enforced. + +5. **The legacy `akerr_name_for_status(status, name)` set path cannot identify + its caller,** so it only checks that *some* reservation covers the status. + It is kept for migration. Consider deprecating it once consumers have moved + to `akerr_register_status_name()`. + +## Unrelated pre-existing issues + +- The `AKERR_USE_STDLIB=OFF` build does not compile at all: `bool`, `PATH_MAX` + and `NULL` are used unconditionally but only included under the stdlib branch. + The README's dependency list is now accurate about what a replacement must + provide, but the header still needs the includes untangled for that + configuration to work. +- `CMakeLists.txt` still sets `main_lib_dest` from `MY_LIBRARY_VERSION`, which is + never defined and never read. Dead line. diff --git a/include/akerror.tmpl.h b/include/akerror.tmpl.h index 56bab3f..26f4b57 100644 --- a/include/akerror.tmpl.h +++ b/include/akerror.tmpl.h @@ -39,10 +39,37 @@ #define AKERR_NOT_IMPLEMENTED (AKERR_LAST_ERRNO_VALUE + 16) /** A method was called that is defined but not currently implemented */ #define AKERR_BADEXC (AKERR_LAST_ERRNO_VALUE + 17) /** The libakerr library was given an akerr_ErrorContext to parse that did not come from AKERR_ARRAY_ERROR (likely an uninitialized pointer) */ -#define AKERR_STATUS_RANGE_OK 0 -#define AKERR_STATUS_RANGE_OVERLAP 1 -#define AKERR_STATUS_RANGE_FULL 2 -#define AKERR_STATUS_RANGE_INVALID 3 +/* + * Status values 0 through 255 are reserved by libakerror at akerr_init() time: + * the host's errno values plus the AKERR_* codes above. Consumers allocate from + * 256 upwards. Reserving any part of this band fails with + * AKERR_STATUS_RANGE_OVERLAP naming AKERR_LIBRARY_OWNER. + */ +#define AKERR_LIBRARY_OWNER "libakerror" +#define AKERR_RESERVED_STATUS_COUNT 256 +#define AKERR_FIRST_CONSUMER_STATUS AKERR_RESERVED_STATUS_COUNT + +/* Results of akerr_reserve_status_range(). */ +#define AKERR_STATUS_RANGE_OK 0 /** The range is now reserved by the caller */ +#define AKERR_STATUS_RANGE_OVERLAP 1 /** Some part of the range is already owned by someone else */ +#define AKERR_STATUS_RANGE_FULL 2 /** No reservation slots remain (see AKERR_MAX_RESERVED_STATUS_RANGES) */ +#define AKERR_STATUS_RANGE_INVALID 3 /** Bad count, bad owner string, or the range overflows int */ + +/* Results of akerr_register_status_name(). */ +#define AKERR_STATUS_NAME_OK 0 /** The name is registered */ +#define AKERR_STATUS_NAME_UNRESERVED 1 /** No owner has reserved a range containing this status */ +#define AKERR_STATUS_NAME_FOREIGN 2 /** The status lies in a range reserved by a different owner */ +#define AKERR_STATUS_NAME_FULL 3 /** The name registry is full (raise AKERR_STATUS_NAME_SLOTS) */ +#define AKERR_STATUS_NAME_INVALID 4 /** NULL/empty/over-long owner, or a NULL name */ + +/* + * The library reserves status values 0 through 255 for itself (see akerr_init), + * which must contain every AKERR_* code above. AKERR_LAST_ERRNO_VALUE is + * derived from the host's errno list at build time, so on a platform with an + * unusually large errno space these codes could escape the band and collide + * with consumer codes allocated at 256. Fail the build instead. + */ +typedef char akerr_assert_codes_within_reserved_band[(AKERR_BADEXC < 256) ? 1 : -1]; #define AKERR_MAX_ARRAY_ERROR 128 @@ -74,7 +101,29 @@ extern akerr_ErrorContext *__akerr_last_ignored; akerr_ErrorContext AKERR_NOIGNORE *akerr_release_error(akerr_ErrorContext *ptr); akerr_ErrorContext AKERR_NOIGNORE *akerr_next_error(); +/* + * Look up (name == NULL) or register (name != NULL) the display name for a + * status. Registration succeeds only if some owner has reserved a range + * containing `status`; prefer akerr_register_status_name(), which also checks + * that the range belongs to you and reports why a registration was refused. + * Never returns NULL -- an unregistered status reads back as "Unknown Error". + */ char *akerr_name_for_status(int status, char *name); + +/* + * Register a display name for a status you own. Returns AKERR_STATUS_NAME_OK, + * or one of the AKERR_STATUS_NAME_* codes above; every refusal is also logged + * through akerr_log_method. `owner` must match the owner string passed to + * akerr_reserve_status_range() for the range containing `status`. + */ +int akerr_register_status_name(const char *owner, int status, const char *name); + +/* + * Claim `count` status values starting at `first_status` for `owner`. Repeating + * an identical reservation for the same owner is a no-op; any other collision + * is refused and logged. Returns one of the AKERR_STATUS_RANGE_* codes; treat + * anything other than AKERR_STATUS_RANGE_OK as an initialization failure. + */ int akerr_reserve_status_range(int first_status, int count, const char *owner); void akerr_init(); void akerr_default_handler_unhandled_error(akerr_ErrorContext *ptr); diff --git a/scripts/generrno.sh b/scripts/generrno.sh index df8e610..247db93 100644 --- a/scripts/generrno.sh +++ b/scripts/generrno.sh @@ -14,7 +14,7 @@ errno --list | while read LINE; do define=$(echo "$LINE" | cut -d ' ' -f 1); value=$(echo "$LINE" | cut -d ' ' -f 2); desc=$(echo "$LINE" | cut -d ' ' -f 3-); - echo " akerr_name_for_status(${define}, \"${desc}\");" >> ${outdir}/src/errno.c ; + echo " akerr_register_status_name(AKERR_LIBRARY_OWNER, ${define}, \"${desc}\");" >> ${outdir}/src/errno.c ; done; echo "}" >> ${outdir}/src/errno.c sed "s/#define AKERR_LAST_ERRNO_VALUE .*/#define AKERR_LAST_ERRNO_VALUE ${maxval}/" ${srcdir}/include/akerror.tmpl.h > ${outdir}/include/akerror.h diff --git a/src/error.c b/src/error.c index 44ff506..1b7958a 100644 --- a/src/error.c +++ b/src/error.c @@ -10,13 +10,43 @@ akerr_ErrorContext *__akerr_last_ignored; akerr_ErrorUnhandledErrorHandler akerr_handler_unhandled_error; akerr_ErrorLogFunction akerr_log_method = NULL; -#define AKERR_MAX_REGISTERED_STATUS_NAMES 512 +/* + * Status-name registry. + * + * Storage is an open-addressed hash table keyed by status value. Both sizes are + * private to this translation unit -- they are deliberately NOT in the public + * header, because a consumer-visible table bound is exactly the ABI hazard this + * registry replaced. Overriding them changes only this file, so a library and + * its consumers can never disagree about the layout. + * + * The table is never resized or rehashed, so a pointer handed out by + * akerr_name_for_status() stays valid for the life of the process. Entries are + * never removed, so probing needs no tombstones. + */ +#ifndef AKERR_STATUS_NAME_SLOTS +#define AKERR_STATUS_NAME_SLOTS 4096 +#endif + +#ifndef AKERR_MAX_RESERVED_STATUS_RANGES #define AKERR_MAX_RESERVED_STATUS_RANGES 64 +#endif + +/* Probing masks with SLOTS-1, so the slot count must be a power of two. This is + * the C99-portable spelling of a static assertion (a negative array bound). */ +typedef char akerr_assert_name_slots_pow2[ + (AKERR_STATUS_NAME_SLOTS > 0 && + (AKERR_STATUS_NAME_SLOTS & (AKERR_STATUS_NAME_SLOTS - 1)) == 0) ? 1 : -1]; + +/* Cap occupancy at 75% so linear probing always meets an empty slot. */ +#define AKERR_MAX_REGISTERED_STATUS_NAMES \ + (AKERR_STATUS_NAME_SLOTS - (AKERR_STATUS_NAME_SLOTS / 4)) + #define AKERR_MAX_STATUS_RANGE_OWNER_LENGTH 64 typedef struct { int status; + int used; char name[AKERR_MAX_ERROR_NAME_LENGTH]; } akerr_StatusName; @@ -27,7 +57,7 @@ typedef struct char owner[AKERR_MAX_STATUS_RANGE_OWNER_LENGTH]; } akerr_StatusRange; -static akerr_StatusName akerr_status_names[AKERR_MAX_REGISTERED_STATUS_NAMES]; +static akerr_StatusName akerr_status_names[AKERR_STATUS_NAME_SLOTS]; static int akerr_status_name_count; static akerr_StatusRange akerr_status_ranges[AKERR_MAX_RESERVED_STATUS_RANGES]; static int akerr_status_range_count; @@ -36,10 +66,19 @@ akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR]; static void akerr_copy_string(char *destination, int capacity, const char *source) { + if ( capacity <= 0 ) { + return; + } strncpy(destination, source, (size_t)capacity - 1); destination[capacity - 1] = '\0'; } +/* + * Compare against each element address rather than testing the address range. + * A range test also accepts pointers into the *interior* of an element, which + * would then be treated as the head of an akerr_ErrorContext and written + * through. Keep this an element-wise scan; it is not a missed optimization. + */ int akerr_valid_error_address(akerr_ErrorContext *ptr) { // Is this within the memory region occupied by AKERR_ARRAY_ERROR? @@ -67,10 +106,18 @@ void akerr_default_logger(const char *fmt, ...) #endif } +/* + * Idempotent. `inited` is set before any work so that the registry calls below + * -- and the public registry entry points, which all call akerr_init() so that + * a consumer reserving its range before anything else touches the library + * cannot have that reservation wiped by a later first-use of the pool -- see + * themselves as already initialized instead of recursing. + */ void akerr_init() { static int inited = 0; if ( inited == 0 ) { + inited = 1; for (int i = 0; i < AKERR_MAX_ARRAY_ERROR; i++ ) { memset((void *)&AKERR_ARRAY_ERROR[i], 0x00, sizeof(akerr_ErrorContext)); AKERR_ARRAY_ERROR[i].arrayid = i; @@ -88,26 +135,33 @@ void akerr_init() akerr_status_name_count = 0; akerr_status_range_count = 0; - /* errno and AKERR_* values are the library-owned compatibility band. */ - (void)akerr_reserve_status_range(0, 256, "libakerror"); + /* errno and AKERR_* values are the library-owned compatibility band. + * This must precede every registration below: naming a status is only + * permitted inside a reserved range. */ + (void)akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, + AKERR_LIBRARY_OWNER); - akerr_name_for_status(AKERR_NULLPOINTER, "Null Pointer Error"); - akerr_name_for_status(AKERR_OUTOFBOUNDS, "Out Of Bounds Error"); - akerr_name_for_status(AKERR_API, "API Error"); - akerr_name_for_status(AKERR_ATTRIBUTE, "Attribute Error"); - akerr_name_for_status(AKERR_TYPE, "Type Error"); - akerr_name_for_status(AKERR_KEY, "Key Error"); - akerr_name_for_status(AKERR_INDEX, "Index Error"); - akerr_name_for_status(AKERR_FORMAT, "Format Error"); - akerr_name_for_status(AKERR_IO, "Input Output Error"); - akerr_name_for_status(AKERR_VALUE, "Value Error"); - akerr_name_for_status(AKERR_RELATIONSHIP, "Relationship Error"); - akerr_name_for_status(AKERR_CIRCULAR_REFERENCE, "Circular Reference Error"); - akerr_name_for_status(AKERR_BADEXC, "Invalid akerr_ErrorContext"); + /* Every AKERR_* code gets a name; tests/err_error_names.c asserts the + * list is exhaustive so a new code cannot be added without one. */ + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_NULLPOINTER, "Null Pointer Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_OUTOFBOUNDS, "Out Of Bounds Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_API, "API Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_ATTRIBUTE, "Attribute Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_TYPE, "Type Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_KEY, "Key Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_INDEX, "Index Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_FORMAT, "Format Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_IO, "Input Output Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_VALUE, "Value Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_RELATIONSHIP, "Relationship Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_EOF, "End Of File"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_CIRCULAR_REFERENCE, "Circular Reference Error"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_ITERATOR_BREAK, "Iterator Break"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_NOT_IMPLEMENTED, "Not Implemented"); + akerr_register_status_name(AKERR_LIBRARY_OWNER, AKERR_BADEXC, "Invalid akerr_ErrorContext"); #if (defined(AKERR_USE_STDLIB) && AKERR_USE_STDLIB == 1) || (!defined(AKERR_USE_STDLIB)) akerr_init_errno(); #endif - inited = 1; } } @@ -150,31 +204,158 @@ akerr_ErrorContext *akerr_release_error(akerr_ErrorContext *err) } +/* + * Scatter the status across the table. Status values are typically dense runs + * (errno 1..N, then a library's block at its base), which linear probing on the + * raw value would pile into one cluster, so mix the bits first. + */ +static unsigned akerr_status_hash(int status) +{ + unsigned h = (unsigned)status; + + h ^= h >> 16; + h *= 0x85ebca6bu; + h ^= h >> 13; + h *= 0xc2b2ae35u; + h ^= h >> 16; + return h & (unsigned)(AKERR_STATUS_NAME_SLOTS - 1); +} + +/* + * Find the slot holding `status`. With create != 0, claim a free slot for it if + * it is not present yet. Returns NULL when the status is absent and either no + * slot was requested or the registry is full. + * + * The `& (AKERR_STATUS_NAME_SLOTS - 1)` below is load-bearing and fails + * silently: off by one in either direction and the probe indexes past + * akerr_status_names, writing into whatever BSS follows rather than crashing. + * A test that only counts how many names registered before the table filled + * cannot see that -- a probe sequence collapsed to two slots still registers + * "some" names. Any change to the probe sequence, the occupancy cap, or the + * power-of-two assumption needs a test that reads every entry back by its own + * distinct value; tests/err_maxval.c does. + */ +static akerr_StatusName *akerr_status_slot(int status, int create) +{ + unsigned slot = akerr_status_hash(status); + + for ( int probe = 0; probe < AKERR_STATUS_NAME_SLOTS; probe++ ) { + akerr_StatusName *entry = &akerr_status_names[slot]; + + if ( entry->used == 0 ) { + if ( create == 0 || + akerr_status_name_count >= AKERR_MAX_REGISTERED_STATUS_NAMES ) { + return NULL; + } + entry->used = 1; + entry->status = status; + entry->name[0] = '\0'; + akerr_status_name_count++; + return entry; + } + if ( entry->status == status ) { + return entry; + } + slot = (slot + 1u) & (unsigned)(AKERR_STATUS_NAME_SLOTS - 1); + } + /* Unreachable: occupancy is capped below the slot count, so the probe above + * always meets a free slot. Present so a future change to that cap cannot + * turn this into a runaway loop. */ + return NULL; +} + +/* The reservation covering `status`, or NULL if nobody has claimed it. */ +static akerr_StatusRange *akerr_range_for_status(int status) +{ + for ( int i = 0; i < akerr_status_range_count; i++ ) { + if ( status >= akerr_status_ranges[i].first && + status <= akerr_status_ranges[i].last ) { + return &akerr_status_ranges[i]; + } + } + return NULL; +} + +/* + * Shared body of both registration entry points. A NULL owner means the caller + * did not identify itself (the legacy two-argument akerr_name_for_status path): + * the status must still lie inside *some* reservation, but we cannot check that + * it is the caller's. Every refusal is logged -- a name that silently fails to + * register degrades into "Unknown Error" in stack traces, which is exactly the + * kind of quiet loss this registry exists to prevent. + */ +static int akerr_store_status_name(const char *owner, int status, const char *name) +{ + akerr_StatusRange *range; + akerr_StatusName *entry; + + if ( name == NULL ) { + return AKERR_STATUS_NAME_INVALID; + } + if ( owner != NULL && ( owner[0] == '\0' || + strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH ) ) { + return AKERR_STATUS_NAME_INVALID; + } + + range = akerr_range_for_status(status); + if ( range == NULL ) { + if ( akerr_log_method != NULL ) { + akerr_log_method("Refusing to name status %d (\"%s\") for %s: no " + "reserved range contains it. Call " + "akerr_reserve_status_range() first.\n", + status, name, owner == NULL ? "an unnamed caller" : owner); + } + return AKERR_STATUS_NAME_UNRESERVED; + } + if ( owner != NULL && strcmp(owner, range->owner) != 0 ) { + if ( akerr_log_method != NULL ) { + akerr_log_method("Refusing to name status %d (\"%s\") for %s: that " + "status is in range %d..%d owned by %s.\n", + status, name, owner, + range->first, range->last, range->owner); + } + return AKERR_STATUS_NAME_FOREIGN; + } + + entry = akerr_status_slot(status, 1); + if ( entry == NULL ) { + if ( akerr_log_method != NULL ) { + akerr_log_method("Status name registry is full (%d entries); " + "dropping name \"%s\" for status %d. Rebuild " + "libakerror with a larger AKERR_STATUS_NAME_SLOTS.\n", + AKERR_MAX_REGISTERED_STATUS_NAMES, name, status); + } + return AKERR_STATUS_NAME_FULL; + } + akerr_copy_string(entry->name, AKERR_MAX_ERROR_NAME_LENGTH, name); + return AKERR_STATUS_NAME_OK; +} + +/* Register a name for a status inside a range the caller reserved. */ +int akerr_register_status_name(const char *owner, int status, const char *name) +{ + akerr_init(); + if ( owner == NULL ) { + return AKERR_STATUS_NAME_INVALID; + } + return akerr_store_status_name(owner, status, name); +} + /* Return or set a name. Status magnitude is unrelated to storage size. */ char *akerr_name_for_status(int status, char *name) { - int i; + akerr_StatusName *entry; - for ( i = 0; i < akerr_status_name_count; i++ ) { - if ( akerr_status_names[i].status == status ) { - if ( name != NULL ) { - akerr_copy_string(akerr_status_names[i].name, - AKERR_MAX_ERROR_NAME_LENGTH, name); - } - return akerr_status_names[i].name; - } + akerr_init(); + if ( name != NULL && + akerr_store_status_name(NULL, status, name) != AKERR_STATUS_NAME_OK ) { + return "Unknown Error"; } - if ( name != NULL ) { - if ( akerr_status_name_count == AKERR_MAX_REGISTERED_STATUS_NAMES ) { - return "Unknown Error"; - } - i = akerr_status_name_count++; - akerr_status_names[i].status = status; - akerr_copy_string(akerr_status_names[i].name, - AKERR_MAX_ERROR_NAME_LENGTH, name); - return akerr_status_names[i].name; + entry = akerr_status_slot(status, 0); + if ( entry == NULL ) { + return "Unknown Error"; } - return "Unknown Error"; + return entry->name; } /* Reserve an inclusive status interval and reject collisions. */ @@ -182,6 +363,7 @@ int akerr_reserve_status_range(int first_status, int count, const char *owner) { int last_status; + akerr_init(); if ( count <= 0 || owner == NULL || owner[0] == '\0' || strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH || first_status > INT_MAX - (count - 1) ) { @@ -209,6 +391,12 @@ int akerr_reserve_status_range(int first_status, int count, const char *owner) } } if ( akerr_status_range_count == AKERR_MAX_RESERVED_STATUS_RANGES ) { + if ( akerr_log_method != NULL ) { + akerr_log_method("Status range table is full (%d ranges); " + "refusing %d..%d for %s.\n", + AKERR_MAX_RESERVED_STATUS_RANGES, + first_status, last_status, owner); + } return AKERR_STATUS_RANGE_FULL; } diff --git a/tests/MUTATION.md b/tests/MUTATION.md index c73b117..05defb4 100644 --- a/tests/MUTATION.md +++ b/tests/MUTATION.md @@ -94,7 +94,7 @@ Re-run after adding tests and confirm the score went up. ## Current status -`src/error.c` scores ~74% (the CI gate is set to 65% for headroom). The +`src/error.c` scores ~77% (the CI gate is set to 65% for headroom). The remaining survivors are dominated by: * **Equivalent mutants** in `akerr_init`: deleting the `memset`/`NULL` setup of @@ -106,7 +106,27 @@ remaining survivors are dominated by: `errctx == NULL` branch, `exit(1)`): killing these needs a subprocess-based test that captures a child's stderr and exit code, rather than the in-process capturing logger the other tests use. +* **Static assertions** (`akerr_assert_name_slots_pow2` and the occupancy cap + it guards): a mutated compile-time assertion that still compiles has no + runtime behavior to observe. Unkillable by construction — the assertion is + itself the test, and `tests/err_maxval.c` covers the runtime consequence. +* **Hash and probe details** in `akerr_status_slot`: dropping one of the + multiply steps in `akerr_status_hash` leaves a worse but still correct hash, + and probing backwards (`slot - 1u`) is an equally valid sequence over a + power-of-two table. Both are behaviorally equivalent. +* **The `capacity <= 0` guard** in `akerr_copy_string`, which is defensive: both + call sites pass a positive constant. Findings surfaced by mutation testing: -* **Superseded:** status names now use a private sparse registry, so the old public `AKERR_MAX_ERR_VALUE` ceiling and its consumer ABI mismatch no longer exist. `tests/err_maxval.c` covers arbitrary `int` values and registry exhaustion. +* **Superseded:** status names now use a private sparse registry, so the old + public `AKERR_MAX_ERR_VALUE` ceiling and its consumer ABI mismatch no longer + exist. `tests/err_maxval.c` covers arbitrary `int` values and registry + exhaustion. +* **Fixed:** the open-addressing probe mask (`& (AKERR_STATUS_NAME_SLOTS - 1)`) + could be mutated to `- 0` or `+ 1` — both of which index past the end of the + table — without any test noticing. `tests/err_maxval.c` only asserted that + *some* names registered before the table filled, which a collapsed probe + sequence still satisfies. It now requires a substantial number of entries and + reads every one of them back by its own distinct name, so a probe that + revisits slots fails on both counts. diff --git a/tests/err_error_names.c b/tests/err_error_names.c index c987935..8ea41c3 100644 --- a/tests/err_error_names.c +++ b/tests/err_error_names.c @@ -7,9 +7,12 @@ * Verify the names are actually installed (mutation testing showed the * registration calls could be deleted without any test noticing). * - * Note: AKERR_EOF, AKERR_ITERATOR_BREAK and AKERR_NOT_IMPLEMENTED are omitted -- - * they are valid codes but akerr_init does not register a display name for them, - * so akerr_name_for_status returns an empty string rather than a known name. + * This list must stay exhaustive. AKERR_EOF, AKERR_ITERATOR_BREAK and + * AKERR_NOT_IMPLEMENTED were previously valid codes with no registered name, so + * they rendered as "Unknown Error" in every stack trace that carried them -- + * the same class of silent gap that a too-small AKERR_MAX_ERR_VALUE used to + * cause. The sweep below walks the whole AKERR_* offset span so a newly added + * code without a name fails here rather than showing up in production traces. */ static const struct { @@ -27,7 +30,10 @@ static const struct { { AKERR_IO, "Input Output Error" }, { AKERR_VALUE, "Value Error" }, { AKERR_RELATIONSHIP, "Relationship Error" }, + { AKERR_EOF, "End Of File" }, { AKERR_CIRCULAR_REFERENCE, "Circular Reference Error" }, + { AKERR_ITERATOR_BREAK, "Iterator Break" }, + { AKERR_NOT_IMPLEMENTED, "Not Implemented" }, { AKERR_BADEXC, "Invalid akerr_ErrorContext" }, }; @@ -41,6 +47,28 @@ int main(void) AKERR_CHECK(strcmp(nm, expected[i].name) == 0); } + /* + * Every value in the library's own offset span must resolve to a real name. + * AKERR_LAST_ERRNO_VALUE + 7 is the one deliberate hole (a removed code); + * anything else nameless is a code someone added without registering it. + */ + for ( int offset = 1; offset <= AKERR_BADEXC - AKERR_LAST_ERRNO_VALUE; offset++ ) { + int code = AKERR_LAST_ERRNO_VALUE + offset; + char *nm = akerr_name_for_status(code, NULL); + if ( offset == 7 ) { + AKERR_CHECK(strcmp(nm, "Unknown Error") == 0); + continue; + } + if ( strcmp(nm, "Unknown Error") == 0 || nm[0] == '\0' ) { + fprintf(stderr, "AKERR_LAST_ERRNO_VALUE + %d (%d) has no name\n", + offset, code); + return 1; + } + } + + /* Every AKERR_* code must sit inside the band the library reserves. */ + AKERR_CHECK(AKERR_BADEXC < AKERR_FIRST_CONSUMER_STATUS); + fprintf(stderr, "err_error_names ok\n"); return 0; } diff --git a/tests/err_maxval.c b/tests/err_maxval.c index 3849e24..5ed8e61 100644 --- a/tests/err_maxval.c +++ b/tests/err_maxval.c @@ -3,26 +3,46 @@ #include #include -/* Status magnitude is no longer coupled to a public array bound. */ +/* + * Status magnitude is no longer coupled to a public array bound: any int is a + * legal status, and storage is a private sparse registry. What bounds the + * registry now is its *capacity*, not the value of the largest code. + * + * Covers: arbitrary int status values, name truncation, range reservation + * semantics (overlap, idempotency, endpoints, validation, overflow), and both + * capacity limits -- the range table and the name table -- each of which must + * report the failure rather than dropping the registration quietly. + */ + int main(void) { akerr_capture_install(); akerr_init(); - AKERR_CHECK(strcmp(akerr_name_for_status(INT_MAX, "Maximum Status"), - "Maximum Status") == 0); - AKERR_CHECK(strcmp(akerr_name_for_status(INT_MIN, "Minimum Status"), - "Minimum Status") == 0); - AKERR_CHECK(strcmp(akerr_name_for_status(INT_MAX, NULL), - "Maximum Status") == 0); - AKERR_CHECK(strcmp(akerr_name_for_status(INT_MIN, NULL), - "Minimum Status") == 0); + /* Any int is a legal status, at either extreme of the range. */ + AKERR_CHECK(akerr_reserve_status_range(INT_MIN, 1, "min-owner") == + AKERR_STATUS_RANGE_OK); + AKERR_CHECK(akerr_reserve_status_range(INT_MAX, 1, "max-owner") == + AKERR_STATUS_RANGE_OK); + AKERR_CHECK(akerr_register_status_name("max-owner", INT_MAX, "Maximum Status") == + AKERR_STATUS_NAME_OK); + AKERR_CHECK(akerr_register_status_name("min-owner", INT_MIN, "Minimum Status") == + AKERR_STATUS_NAME_OK); + AKERR_CHECK(strcmp(akerr_name_for_status(INT_MAX, NULL), "Maximum Status") == 0); + AKERR_CHECK(strcmp(akerr_name_for_status(INT_MIN, NULL), "Minimum Status") == 0); + + /* A name longer than the buffer is truncated and always terminated. */ + AKERR_CHECK(akerr_reserve_status_range(1000000, 1, "trunc") == + AKERR_STATUS_RANGE_OK); const char *long_name = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-extra"; - char *stored = akerr_name_for_status(1000000, (char *)long_name); + AKERR_CHECK(akerr_register_status_name("trunc", 1000000, long_name) == + AKERR_STATUS_NAME_OK); + char *stored = akerr_name_for_status(1000000, NULL); AKERR_CHECK(strlen(stored) == AKERR_MAX_ERROR_NAME_LENGTH - 1); AKERR_CHECK(stored[AKERR_MAX_ERROR_NAME_LENGTH - 1] == '\0'); + /* Reservation: overlap detection, and idempotency for an exact repeat. */ AKERR_CHECK(akerr_reserve_status_range(256, 16, "component-a") == AKERR_STATUS_RANGE_OK); AKERR_CHECK(akerr_reserve_status_range(256, 16, "component-a") == @@ -30,8 +50,15 @@ int main(void) AKERR_CHECK(akerr_reserve_status_range(260, 2, "component-b") == AKERR_STATUS_RANGE_OVERLAP); AKERR_CHECK_CONTAINS("component-a"); + + /* The library's own 0..255 band is reserved and cannot be encroached on. */ AKERR_CHECK(akerr_reserve_status_range(255, 1, "component-b") == AKERR_STATUS_RANGE_OVERLAP); + AKERR_CHECK(akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, + AKERR_LIBRARY_OWNER) == + AKERR_STATUS_RANGE_OK); /* exact repeat by the owner */ + + /* Argument validation. */ AKERR_CHECK(akerr_reserve_status_range(INT_MAX, 2, "overflow") == AKERR_STATUS_RANGE_INVALID); AKERR_CHECK(akerr_reserve_status_range(300, 0, "empty") == @@ -43,6 +70,7 @@ int main(void) AKERR_CHECK(akerr_reserve_status_range(300, 1, "") == AKERR_STATUS_RANGE_INVALID); + /* Owner strings: 63 chars fit, 64 do not. */ char owner63[AKERR_MAX_ERROR_NAME_LENGTH]; char owner64[AKERR_MAX_ERROR_NAME_LENGTH + 1]; memset(owner63, 'a', sizeof(owner63) - 1); @@ -53,37 +81,79 @@ int main(void) AKERR_STATUS_RANGE_OK); AKERR_CHECK(akerr_reserve_status_range(401, 1, owner64) == AKERR_STATUS_RANGE_INVALID); - AKERR_CHECK(akerr_reserve_status_range(INT_MAX, 1, "int-max") == - AKERR_STATUS_RANGE_OK); + /* Partial overlaps at either endpoint, and a same-range different owner. */ AKERR_CHECK(akerr_reserve_status_range(500, 2, "endpoint") == AKERR_STATUS_RANGE_OK); AKERR_CHECK(akerr_reserve_status_range(499, 2, "left") == AKERR_STATUS_RANGE_OVERLAP); AKERR_CHECK(akerr_reserve_status_range(500, 1, "endpoint") == - AKERR_STATUS_RANGE_OVERLAP); + AKERR_STATUS_RANGE_OVERLAP); /* subset, not an exact repeat */ AKERR_CHECK(akerr_reserve_status_range(501, 1, "endpoint") == AKERR_STATUS_RANGE_OVERLAP); AKERR_CHECK(akerr_reserve_status_range(500, 2, "other") == - AKERR_STATUS_RANGE_OVERLAP); + AKERR_STATUS_RANGE_OVERLAP); /* same range, wrong owner */ - /* Five ranges exist: library, component-a, owner63, INT_MAX, endpoint. */ - for ( int i = 0; i < 59; i++ ) { - AKERR_CHECK(akerr_reserve_status_range(1000 + (i * 2), 1, "fill") == - AKERR_STATUS_RANGE_OK); + /* Claim room for the name-exhaustion sweep before filling the range table. */ + AKERR_CHECK(akerr_reserve_status_range(2000000, 100000, "fill") == + AKERR_STATUS_RANGE_OK); + + /* + * Range table capacity. The limit is private to src/error.c on purpose, so + * discover it by filling rather than by hardcoding it here. + */ + akerr_capture_reset(); + int ranges_added = 0; + int range_rc = AKERR_STATUS_RANGE_OK; + for ( int i = 0; i < 100000; i++ ) { + range_rc = akerr_reserve_status_range(1000 + (i * 2), 1, "pad"); + if ( range_rc != AKERR_STATUS_RANGE_OK ) { + break; + } + ranges_added++; } - AKERR_CHECK(akerr_reserve_status_range(2000, 1, "too-many") == - AKERR_STATUS_RANGE_FULL); + AKERR_CHECK(ranges_added > 0); + AKERR_CHECK(range_rc == AKERR_STATUS_RANGE_FULL); + AKERR_CHECK_CONTAINS("range table is full"); - int filled = 0; - for ( int status = 2000000; status < 2000600; status++ ) { - if ( strcmp(akerr_name_for_status(status, "Filled"), "Filled") != 0 ) { - filled = 1; + /* + * Name table capacity. Exhaustion must be reported, not silent: a dropped + * name degrades every future stack trace for that code to "Unknown Error". + */ + akerr_capture_reset(); + int full_at = -1; + for ( int i = 0; i < 100000; i++ ) { + char name[32]; + snprintf(name, sizeof(name), "Filled %d", i); + int rc = akerr_register_status_name("fill", 2000000 + i, name); + if ( rc != AKERR_STATUS_NAME_OK ) { + AKERR_CHECK(rc == AKERR_STATUS_NAME_FULL); + full_at = i; break; } } - AKERR_CHECK(filled == 1); + AKERR_CHECK_CONTAINS("registry is full"); + AKERR_CHECK_CONTAINS("AKERR_STATUS_NAME_SLOTS"); - fprintf(stderr, "err_maxval ok\n"); + /* + * The table must actually hold everything it accepted. A probe sequence + * that revisits slots instead of walking the table -- e.g. masking with + * SLOTS rather than SLOTS-1 -- both collapses the usable capacity and + * loses earlier entries, and each check below catches it independently. + * The floor assumes at least the default table size (4096 slots). + */ + AKERR_CHECK(full_at > 256); + for ( int i = 0; i < full_at; i++ ) { + char expected[32]; + snprintf(expected, sizeof(expected), "Filled %d", i); + AKERR_CHECK(strcmp(akerr_name_for_status(2000000 + i, NULL), expected) == 0); + } + + /* A dropped name reads back as the sentinel, and earlier ones survive. */ + AKERR_CHECK(strcmp(akerr_name_for_status(2000000 + full_at, NULL), + "Unknown Error") == 0); + AKERR_CHECK(strcmp(akerr_name_for_status(INT_MIN, NULL), "Minimum Status") == 0); + + fprintf(stderr, "err_maxval ok (%d consumer names before full)\n", full_at); return 0; } diff --git a/tests/err_name_ownership.c b/tests/err_name_ownership.c new file mode 100644 index 0000000..bd58458 --- /dev/null +++ b/tests/err_name_ownership.c @@ -0,0 +1,97 @@ +#include "akerror.h" +#include "err_capture.h" +#include + +/* + * Reserving a range used to be pure bookkeeping: akerr_name_for_status() would + * name any status for any caller, so two components could still register names + * for the same code -- and HANDLE the same code -- with nothing detecting it. + * Reservation only caught components that both opted in AND declared ranges + * that happened to overlap. + * + * Naming a status is now permitted only inside a reservation: + * - akerr_register_status_name() requires the range to belong to the caller; + * - the legacy two-argument akerr_name_for_status() set path cannot identify + * its caller, so it can only require that *some* reservation covers the + * status -- still enough to stop a code nobody claimed. + * Every refusal is logged, because a name that fails to register degrades the + * status to "Unknown Error" in every later stack trace. + */ + +int main(void) +{ + akerr_capture_install(); + akerr_init(); + + AKERR_CHECK(akerr_reserve_status_range(256, 16, "lib-a") == + AKERR_STATUS_RANGE_OK); + AKERR_CHECK(akerr_reserve_status_range(512, 16, "lib-b") == + AKERR_STATUS_RANGE_OK); + + /* The owner of a range may name statuses inside it. */ + AKERR_CHECK(akerr_register_status_name("lib-a", 256, "A Parse Error") == + AKERR_STATUS_NAME_OK); + AKERR_CHECK(akerr_register_status_name("lib-a", 271, "A Last Error") == + AKERR_STATUS_NAME_OK); + AKERR_CHECK(strcmp(akerr_name_for_status(256, NULL), "A Parse Error") == 0); + + /* Naming another owner's status is refused and names the real owner. */ + akerr_capture_reset(); + AKERR_CHECK(akerr_register_status_name("lib-b", 256, "B Hijack") == + AKERR_STATUS_NAME_FOREIGN); + AKERR_CHECK_CONTAINS("lib-a"); + AKERR_CHECK_CONTAINS("lib-b"); + AKERR_CHECK(strcmp(akerr_name_for_status(256, NULL), "A Parse Error") == 0); + + /* Including the library's own reserved band. */ + akerr_capture_reset(); + AKERR_CHECK(akerr_register_status_name("lib-b", AKERR_VALUE, "B Value") == + AKERR_STATUS_NAME_FOREIGN); + AKERR_CHECK_CONTAINS(AKERR_LIBRARY_OWNER); + AKERR_CHECK(strcmp(akerr_name_for_status(AKERR_VALUE, NULL), "Value Error") == 0); + + /* A status nobody reserved cannot be named through either entry point. */ + akerr_capture_reset(); + AKERR_CHECK(akerr_register_status_name("lib-a", 9999, "Unclaimed") == + AKERR_STATUS_NAME_UNRESERVED); + AKERR_CHECK_CONTAINS("no reserved range"); + AKERR_CHECK(strcmp(akerr_name_for_status(9999, NULL), "Unknown Error") == 0); + + akerr_capture_reset(); + AKERR_CHECK(strcmp(akerr_name_for_status(9999, "Unclaimed Legacy"), + "Unknown Error") == 0); + AKERR_CHECK_CONTAINS("no reserved range"); + AKERR_CHECK(strcmp(akerr_name_for_status(9999, NULL), "Unknown Error") == 0); + + /* Boundaries: just outside lib-a's range is not lib-a's to name. */ + AKERR_CHECK(akerr_register_status_name("lib-a", 255, "Below") == + AKERR_STATUS_NAME_FOREIGN); + AKERR_CHECK(akerr_register_status_name("lib-a", 272, "Above") == + AKERR_STATUS_NAME_UNRESERVED); + + /* The legacy set path still works inside any reservation. */ + AKERR_CHECK(strcmp(akerr_name_for_status(513, "B Legacy"), "B Legacy") == 0); + AKERR_CHECK(strcmp(akerr_name_for_status(513, NULL), "B Legacy") == 0); + + /* Re-registering your own status overwrites the name. */ + AKERR_CHECK(akerr_register_status_name("lib-a", 256, "A Renamed") == + AKERR_STATUS_NAME_OK); + AKERR_CHECK(strcmp(akerr_name_for_status(256, NULL), "A Renamed") == 0); + + /* Argument validation. */ + AKERR_CHECK(akerr_register_status_name(NULL, 257, "No Owner") == + AKERR_STATUS_NAME_INVALID); + AKERR_CHECK(akerr_register_status_name("", 257, "Empty Owner") == + AKERR_STATUS_NAME_INVALID); + AKERR_CHECK(akerr_register_status_name("lib-a", 257, NULL) == + AKERR_STATUS_NAME_INVALID); + + /* A refused registration must not consume a slot or leave a partial entry. */ + AKERR_CHECK(strcmp(akerr_name_for_status(257, NULL), "Unknown Error") == 0); + + /* Lookup is unaffected by ownership -- anyone may read any name. */ + AKERR_CHECK(strcmp(akerr_name_for_status(271, NULL), "A Last Error") == 0); + + fprintf(stderr, "err_name_ownership ok\n"); + return 0; +} diff --git a/tests/err_registry_init_order.c b/tests/err_registry_init_order.c new file mode 100644 index 0000000..6529c8b --- /dev/null +++ b/tests/err_registry_init_order.c @@ -0,0 +1,56 @@ +#include "akerror.h" +#include "err_capture.h" +#include + +/* + * A library may reserve its status range from its own init() before anything in + * the process has raised an error, i.e. before akerr_init() has run. That used + * to be silently destructive: akerr_init() clears the range and name tables, so + * whichever component first triggered it (via PREPARE_ERROR) wiped the earlier + * reservation, and the *next* component to claim the same range was told OK -- + * producing exactly the undetected aliasing the registry exists to prevent. + * + * Every public registry entry point now calls akerr_init() itself, so the + * tables are only ever cleared before the first reservation, never after one. + * + * Note this test must not call akerr_init() or PREPARE_ERROR first -- the + * uninitialized entry is the whole point. + */ + +int main(void) +{ + akerr_capture_install(); + + /* Cold call: no akerr_init(), no PREPARE_ERROR anywhere yet. */ + AKERR_CHECK(akerr_reserve_status_range(256, 16, "early-lib") == + AKERR_STATUS_RANGE_OK); + AKERR_CHECK(akerr_register_status_name("early-lib", 256, "Early Error") == + AKERR_STATUS_NAME_OK); + + /* Something else now uses the library for the first time. */ + akerr_init(); + PREPARE_ERROR(e); + (void)e; + + /* The early reservation and its name must both have survived. */ + AKERR_CHECK(strcmp(akerr_name_for_status(256, NULL), "Early Error") == 0); + AKERR_CHECK(akerr_reserve_status_range(256, 16, "late-lib") == + AKERR_STATUS_RANGE_OVERLAP); + AKERR_CHECK_CONTAINS("early-lib"); + AKERR_CHECK(akerr_reserve_status_range(260, 2, "late-lib") == + AKERR_STATUS_RANGE_OVERLAP); + + /* The library's own initialization still happened exactly once. */ + AKERR_CHECK(strcmp(akerr_name_for_status(AKERR_NULLPOINTER, NULL), + "Null Pointer Error") == 0); + AKERR_CHECK(akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT, + AKERR_LIBRARY_OWNER) == + AKERR_STATUS_RANGE_OK); + + /* An identical repeat by the original owner is still idempotent. */ + AKERR_CHECK(akerr_reserve_status_range(256, 16, "early-lib") == + AKERR_STATUS_RANGE_OK); + + fprintf(stderr, "err_registry_init_order ok\n"); + return 0; +}