Files
libakerror/TODO.md
Andrew Kesterson 5695061130
Some checks failed
libakerror CI Build / cmake_build (push) Successful in 2m47s
libakerror CI Build / coverage (push) Successful in 2m48s
libakerror CI Build / thread_sanitizer (push) Failing after 2m49s
libakerror CI Build / mutation_test (push) Successful in 39m56s
Split the README reference material into docs/
The README was 794 lines: the summary, the design rationale, the whole macro
reference, the threading contract, the build internals and the exit-status
specification in one file. It is now 178 lines -- summary, installation,
quickstart, and an index -- and the reference material lives in docs/, one file
per topic: architecture, usage, status-codes, uncaught-errors, exit-status,
thread-safety, building.

The prose moved as written. Inbound references followed it: UPGRADING.md,
TODO.md, include/akerror.tmpl.h and tests/err_threads_handoff.c now name the
docs/ file that owns the text they cite, and AGENTS.md says where new
documentation goes so the README does not grow back.

Five factual errors fixed in the moved text:

- Both NULL-pointer examples inverted their test. FAIL_ZERO_* fails when the
  expression is zero, so `(somePointer == NULL)` failed on a *valid* pointer.
  They now read `(somePointer != NULL)`.
- AKERROR_NOIGNORE, four times including the #define, is AKERR_NOIGNORE.
- FINISH_NORExbTURN is FINISH_NORETURN.
- "functiions" is "functions".
- The architecture link pointed at include/akerror.h, which is generated and
  not in the tree; it points at include/akerror.tmpl.h.

The quickstart is new text. It compiles under -Wall -Wextra -Werror and was run
through all three of its paths: handled usage error exits 0, unhandled IO error
prints a trace and exits with the status, success exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 16:56:07 -04:00

8.4 KiB

TODO

Working notes for libakerror. Outstanding items only.

1. Only ThreadSanitizer is wired into CI, not ASan/UBSan

AKERR_SANITIZE builds the library and the tests with any sanitizer list, and CI runs -DAKERR_SANITIZE=thread through scripts/thread_test.sh. Nothing runs address,undefined yet, and that is the one that covers the original motivation: mutation testing caught an out-of-bounds probe in the status-name hash table that the suite could not, because the failure mode was a write into adjacent BSS, which does not crash. Sharpening one test closed that instance; ASan would catch the whole class regardless of how sharp the assertions are.

The machinery is in place — this is one more job in .gitea/workflows/ci.yaml running cmake -S . -B build/asan -DAKERR_SANITIZE=address,undefined. Left separate because ASan and TSan cannot be combined in one build.

2. HANDLE-level status 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. Ownership 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 — rewriting PROCESS/HANDLE/HANDLE_GROUP/HANDLE_DEFAULT/FINISH so status matching is not restricted to integer constant expressions. That would also allow matching on ranges or predicates, and would let a handler resolve a code through its owner. It touches the most load-bearing code in the library and every consumer's error handling at once, so it wants its own change.

Note it would not by itself fix the "don't use CATCH or FAIL_*_BREAK inside a loop" hazard: that comes from exiting via break, not from switch.

3. No registry introspection

There is no way to ask who owns a status, or to enumerate reservations. The "coordinate ranges at the dependency-stack level" advice in UPGRADING.md therefore 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, additive, and the natural next step for multi-component adoption.

4. No way to release a reservation

A plugin host that dlopens many distinct plugins over a process lifetime accumulates ranges until the table fills. Reloading the same plugin is fine — an identical repeat by the same owner is idempotent.

5. Renaming a status is not safe against a concurrent lookup

akerr_name_for_status(status, NULL) returns a pointer into the registry rather than a copy, which is what makes it usable from inside FAIL — it needs no buffer and no error context of its own. Registering a second name for a status that already has one (tests/err_name_ownership.c covers that it is allowed) overwrites that buffer in place, so a thread reading the name at that moment can see a torn string. Every other registry operation is serialized; this one cannot be, because the reader is outside the lock by the time it reads the characters.

Documented in docs/thread-safety.md and UPGRADING.md as "register names during initialization". Closing it properly means making a registered name immutable — either refusing a rename outright (a behavior change, and tests/err_name_ownership.c asserts the current contract), or copying names into a bump-allocated arena and publishing the pointer with a release store, so a rename allocates new storage instead of rewriting live storage. The arena is the better answer; it costs a second capacity limit and its exhaustion path.

6. Deprecate the two-argument name-registration path

akerr_name_for_status(status, name) cannot identify its caller, so it can only check that some reservation covers the status, not that the caller owns it. It exists for migration. Once consumers have moved to akerr_register_status_name(), make the set path a no-op or remove it and leave akerr_name_for_status() as pure lookup.

7. akerr_init()'s own reservation failure is untested

tests/err_library_status_fatal.c covers the terminal path in __akerr_name_library_status() by naming a status the library does not own. The band reservation in akerr_init() has no such handle: it can only fail in a build whose tables are too small for the library's own entries, and both sizes are PRIVATE to the library target, so a test executable cannot set them.

Closing it means a second library target built with tiny tables plus a WILL_FAIL test linked against it. Nothing in the CMake does that yet: the sanitizer and coverage options vary the flags of the one library target, not its compile definitions.

Related: branch coverage on src/error.c now sits just above its 50% gate. Every FAIL_* site carries about six branch outcomes of error-construction machinery (ENSURE_ERROR_READY, AKERR_STACKTRACE_APPEND) that only run when that specific failure fires, and every PASS site around a call that cannot fail carries about twenty-five. Validating more inputs therefore lowers the ratio by construction. Before adding defensive checks, expect to add a test that drives them, as tests/err_copy_string.c does.

8. Mutation testing judges concurrency mutants without a sanitizer

scripts/mutation_test.py configures each mutant build with the default CMake options, so a mutant that only breaks under concurrency is judged by a suite running without ThreadSanitizer. Deleting the pool's akerr_mutex_lock() call survives the run even though it is a real race: rebuilt and run directly, that mutant fails tests/err_threads_pool.c in 4 of 10 runs, and fails under scripts/thread_test.sh in 5 of 5. So 81.2% is a floor for that category, not a verdict.

Closing it means a --cmake-arg passthrough on the harness so the mutant build can be configured with -DAKERR_SANITIZE=thread. The whole run then costs a TSan-instrumented suite per mutant (roughly 6s instead of 0.4s), so it belongs behind a flag rather than in the default target or in CI.

9. No way to keep an error context and report it at the same time

A context can be handed to another thread and released there -- docs/thread-safety.md now documents that pattern and tests/err_threads_handoff.c proves it -- but it is a move. A thread that wants to both keep its error and report it upward has to read the fields out into its own record, and it loses the stack trace doing so, because stacktracebuf is the one thing that cannot be usefully summarized.

Copying the struct is not a workaround. stacktracebufptr is self-referential (include/akerror.tmpl.h), so akerr_ErrorContext c = *src; leaves the copy's cursor pointing into the source's buffer -- the copy logs correctly and then corrupts a slot it does not own the first time anything appends to it. arrayid is restored after the wipe in akerr_release_error() (src/error.c:395-398), so a copied id makes the destination impersonate the source's slot for the life of the process.

If this is ever worth an API, the shape is:

akerr_ErrorContext AKERR_NOIGNORE *akerr_copy_error(akerr_ErrorContext *source,
						    akerr_ErrorContext *destination);

taking the destination as a parameter rather than allocating it. An allocating copy could fail on pool exhaustion, and reporting that failure needs a pool slot, so it would have to abort -- adding a third exit() site to a library that deliberately has two. Caller-allocates puts the pool pressure where it can be managed. The copy must repair four fields: arrayid (the destination's own), refcount (set to 1, never inherited), handled (false -- a copy is a fresh obligation, or FINISH_NORETURN on the receiving side drops it silently), and stacktracebufptr (re-anchored to the destination's buffer at the same offset, so a later append continues the trace instead of overwriting it).

Not worth building yet: no consumer needs it. The trigger is a consumer that needs a worker's stack trace, not just its status and message, at the join point -- libakstdlib's planned pthread_* wrappers are the likely first.

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. docs/building.md's dependency list states what a replacement must provide, but the header still needs its includes untangled for that configuration to work.
  • CMakeLists.txt sets main_lib_dest from MY_LIBRARY_VERSION, which is never defined and never read. Dead line.