Andrew Kesterson
f1283e21a3
Reservations were advisory bookkeeping: any component could name any status, so the registry only detected declared-range overlap between components that both opted in. Naming a status now requires a reservation. akerr_register_status_name() checks that the range belongs to the caller, and the legacy two-argument akerr_name_for_status() set path, which cannot identify its caller, requires that some reservation covers the status. Every refusal is logged and names the real owner, because a name that fails to register degrades that code to "Unknown Error" in every later stack trace. Fix a reservation made before the first PREPARE_ERROR being silently discarded. akerr_init() clears the tables, so whichever component first triggered it wiped an earlier reservation and the next component to claim the same range was told it was free, producing exactly the undetected aliasing the registry exists to prevent. Every registry entry point now calls akerr_init(), which sets its guard before doing any work so those calls do not recurse. Replace the linear-scan name array with an open-addressed hash table, taking lookup from O(n) to O(1) and raising usable capacity from 512 entries (366 free to consumers after errno registration) to 3072 (~2900 free). Both table sizes are build-time overridable and applied PRIVATE: they live entirely in src/error.c, so raising them cannot desynchronize a library from its consumers the way AKERR_MAX_ERR_VALUE could. Exhausting either table is now logged and returned to the caller rather than silently dropping the entry. No dynamic allocation is introduced; both tables remain file-scope arrays, and the library's undefined-symbol set gains only strcmp and strlen. Register names for AKERR_EOF, AKERR_ITERATOR_BREAK and AKERR_NOT_IMPLEMENTED, which had none and rendered as "Unknown Error" in every stack trace carrying them. err_error_names.c now sweeps the whole AKERR_* offset span so a code added without a name fails there instead of in production traces. Add static assertions that the slot count is a power of two and that AKERR_BADEXC stays inside the library's own 0-255 band, the latter guarding against a host errno space large enough to push library codes into the range consumers are told to allocate from. Set a project version and soname (1.0.0 / libakerror.so.1) so a stale installed library can no longer be silently paired with newer headers, and so akerror.pc ships a real Version field instead of an empty one. Mutation testing surfaced an out-of-bounds probe in the new table that the suite did not catch: masking with SLOTS rather than SLOTS-1 indexes past the array, and err_maxval.c asserted only that some names registered before the table filled, which a collapsed probe sequence still satisfies. It now requires a substantial entry count and reads every entry back by its own distinct name. Tests: 28/28 pass. Coverage 99.4% line / 86.8% branch. Mutation score for src/error.c 74% -> 77.3%. Compatibility: source and ABI break. AKERR_MAX_ERR_VALUE and the __AKERR_ERROR_NAMES data symbol are gone, custom codes must move out of 0-255, and names must be registered against a reserved range. README.md carries the migration steps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
112 lines
6.1 KiB
Markdown
112 lines
6.1 KiB
Markdown
# TODO
|
|
|
|
The status-code ownership work has been consumed. The implementation now:
|
|
|
|
- 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.
|
|
|
|
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.
|