Files
libakerror/TODO.md
Tachikoma 24469e83d4 Move outstanding work from TODO.md into the issue tracker
Fourteen issues on source.starfort.tech/andrew/libakerror, labelled by kind and
blast radius and milestoned by what they can land in: 2.0.x for anything that
breaks no ABI, 2.1.0 for additive surface, 3.0.0 for the handler-ladder rewrite.
Everything filed carries status::grooming.

Three of the fourteen were not in this file at all. They are defects in this
library that were written down in a consumer's TODO instead: IGNORE() leaking a
context (#14), the coverage target not being namespaced when embedded (#15),
and no akerrorConfigVersion.cmake being installed (#16). libakstdlib carries a
workaround for each. That is the finding worth keeping -- a library whose
consumers record its defects in their own files cannot see them, and the
workaround gets written once per consumer.

TODO.md keeps the reasoning: why the handler ladder is a major-version change,
why a copied akerr_ErrorContext is a trap in two specific fields, why
validating more inputs lowers branch coverage by construction, why the mutation
score is a floor, and why a registered name is returned by pointer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Andrew Kesterson <andrew@aklabs.net>
2026-08-02 19:00:22 -04:00

104 lines
5.2 KiB
Markdown

# Record
**Outstanding work is in the issue tracker, not in this file:**
<https://source.starfort.tech/andrew/libakerror/issues>
Issues are labelled by kind and blast radius, and milestoned by what they can land
in: `2.0.x` for anything that breaks no ABI, `2.1.0` for additive surface, `3.0.0`
for the handler-ladder rewrite and the contract changes that go with it. Everything
filed carries `status::grooming` until it has been through grooming.
What stays here is the reasoning a tracker has no place for.
## Three defects were recorded only downstream
Found while moving this file and `libakstdlib`'s into their trackers. Each is a
defect **in this library**, each was written down in a *consumer's* TODO file, and
none had an entry here:
| Issue | What it is | What it costs downstream |
|---|---|---|
| #14 | `IGNORE()` logs a context and never releases it | `libakstdlib`'s `aksl_tree_iterate` open-codes log-then-release by hand; every `IGNORE()` in `libakgl`'s `CLEANUP` blocks is a leaked slot |
| #15 | The `coverage` target is not namespaced when embedded, though `mutation` is | `libakstdlib` shadows `add_custom_target` to embed this library at all |
| #16 | No `akerrorConfigVersion.cmake` is installed | No consumer can ask `find_dependency(akerror)` for a version floor |
**That is the finding worth keeping, more than the three defects.** A library whose
consumers record its defects in their own files has no way to see them: each
consumer knows one, nobody sees the pattern, and the workaround gets written three
times. The tracker is now the place, and a consumer filing upstream costs them one
issue instead of one workaround.
## Why the handler ladder is a `3.0.0` change and not a patch
`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` compile to a `switch`,
which restricts status matching to integer constant expressions. That is what makes
`HANDLE`-level aliasing undetectable (#4): two components can compile the same
integer into a `case` label without reserving a range or registering a name, and
**ownership enforcement never sees it, because it covers naming and the `case` label
never reaches the library.**
Rewriting it as an `if`/`else if` ladder would allow matching on ranges or
predicates and let a handler resolve a code through its owner. It also touches **the
most load-bearing code in the library and every consumer's error handling at once.**
One thing it would *not* fix, recorded so nobody expects it to: the "don't use
`CATCH` or `FAIL_*_BREAK` inside a loop" hazard comes from exiting via `break`, not
from `switch`.
## Why a copied `akerr_ErrorContext` is a trap
Recorded because the struct looks copyable and is not, and #11 exists only if a
consumer ever needs it.
- **`stacktracebufptr` is self-referential.** `akerr_ErrorContext c = *src;` leaves
the copy's cursor pointing into the source's buffer, so the copy logs correctly
and then **corrupts a slot it does not own** the first time anything appends.
- **`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 it is ever built, the shape takes 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 — a third `exit()` site
in a library that deliberately has two.
## Why validating more inputs lowers branch coverage
Branch coverage on `src/error.c` sits just above its 50% gate, and the gate is set
where it is on purpose.
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. **Every `PASS` site around a call that cannot fail carries
about twenty-five.**
So adding a defensive check lowers the ratio by construction. **Before adding one,
expect to add a test that drives it**, as `tests/err_copy_string.c` does.
## Why the mutation score is a floor
`scripts/mutation_test.py` configures each mutant with the default CMake options,
so **a mutant that only breaks under concurrency is judged by a suite running
without ThreadSanitizer.**
Measured: 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.
**81.2% is therefore a floor for that category, not a verdict.** #10 is the
passthrough that would fix the measurement, and it belongs behind a flag: a
TSan-instrumented suite per mutant costs roughly 6s instead of 0.4s.
## Why a registered name is returned by pointer
`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.
The cost is that renaming a status is not safe against a concurrent lookup (#7):
every other registry operation is serialized, and 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".