Reduce TODO.md to outstanding work
The status-code ownership notes described work that has landed. That record belongs in the commit that made the change and in the comments around the code it constrains, not in a file whose purpose is naming what is left. Keeps the six open items and the two unrelated pre-existing issues, and promotes the missing sanitizer run to the top: mutation testing found an out-of-bounds probe whose failure mode was a silent BSS write, which no assertion-based test was positioned to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
140
TODO.md
140
TODO.md
@@ -1,111 +1,73 @@
|
|||||||
# TODO
|
# TODO
|
||||||
|
|
||||||
The status-code ownership work has been consumed. The implementation now:
|
Working notes for `libakerror`. Outstanding items only.
|
||||||
|
|
||||||
- stores status names in a private, open-addressed sparse registry, removing the
|
## 1. The test suite has no sanitizer run
|
||||||
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
|
Mutation testing caught an out-of-bounds probe in the status-name hash table
|
||||||
BSS, and the library's undefined-symbol set remains `exit`, `memset`, `snprintf`,
|
that the suite could not: the failure mode was a write into adjacent BSS, which
|
||||||
`vfprintf`, `strncpy`, `strcmp`, `strlen` and `stderr` — no allocator. The name
|
does not crash, so every test still passed. Sharpening one test closed that
|
||||||
table grew from ~34 KB to 288 KB, against the ~4.55 MB `AKERR_ARRAY_ERROR`
|
instance, but ASan would have caught the whole class directly and independently
|
||||||
already occupies, so total BSS moved ~4.6 MB → ~4.9 MB. Raising
|
of how sharp the assertions are.
|
||||||
`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
|
Add a `-fsanitize=address,undefined` build to `.gitea/workflows/ci.yaml`, or a
|
||||||
range starting at 256 or higher; `libakgl`'s codes at `AKERR_LAST_ERRNO_VALUE +
|
CMake option alongside `AKERR_COVERAGE`. This is the highest-value item here: it
|
||||||
18..22` land inside the reserved band and will fail to reserve.
|
covers the whole library, not just the registry, and the library's fixed pools
|
||||||
|
and manual buffer arithmetic are exactly what it is good at.
|
||||||
|
|
||||||
## Mutation testing surfaced a latent out-of-bounds write in the new registry
|
## 2. `HANDLE`-level status aliasing is still undetectable
|
||||||
|
|
||||||
Worth recording because the failure mode is silent and the class will recur if
|
Two components can compile the same integer into a `case` label without ever
|
||||||
the registry is touched again.
|
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.
|
||||||
|
|
||||||
`akerr_status_slot()` advances its probe with
|
Closing this needs the `if`/`else if` handler ladder — rewriting
|
||||||
`slot = (slot + 1u) & (AKERR_STATUS_NAME_SLOTS - 1)`. Mutating that mask to
|
`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` so status matching
|
||||||
`- 0` or `+ 1` makes it index *past* the end of `akerr_status_names` — a write
|
is not restricted to integer constant expressions. That would also allow
|
||||||
into adjacent BSS. Both mutants **survived the test suite**: `err_maxval.c` only
|
matching on ranges or predicates, and would let a handler resolve a code through
|
||||||
asserted that *some* names registered before the table reported full, and a
|
its owner. It touches the most load-bearing code in the library and every
|
||||||
probe sequence that collapses to two slots still satisfies that. The test now
|
consumer's error handling at once, so it wants its own change.
|
||||||
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:
|
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`.
|
||||||
|
|
||||||
- **That mask is load-bearing and fails silently.** It corrupts neighbouring
|
## 3. No registry introspection
|
||||||
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
|
There is no way to ask who owns a status, or to enumerate reservations. The
|
||||||
|
"coordinate ranges at the dependency-stack level" advice in README.md therefore
|
||||||
|
has no tooling behind it.
|
||||||
|
|
||||||
Ordered by how likely they are to matter as more components adopt the library.
|
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.
|
||||||
|
|
||||||
1. **`HANDLE`-level aliasing is still undetectable.** Two components can compile
|
## 4. No way to release a reservation
|
||||||
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
|
A plugin host that `dlopen`s many distinct plugins over a process lifetime
|
||||||
enumerate reservations, so the "coordinate ranges at the dependency-stack
|
accumulates ranges until the table fills. Reloading the *same* plugin is fine —
|
||||||
level" advice has no tooling behind it. A read-only accessor plus a dump
|
an identical repeat by the same owner is idempotent.
|
||||||
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
|
## 5. The registry is not thread safe
|
||||||
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
|
Global mutable state, no locking, and `akerr_status_name_count++` is not atomic.
|
||||||
`akerr_status_name_count++` is not atomic. Documented as an
|
Currently documented as an initialization-time-only API rather than enforced. If
|
||||||
initialization-time-only API; not enforced.
|
components start initializing on separate threads this needs either a lock or a
|
||||||
|
documented once-per-process init barrier.
|
||||||
|
|
||||||
5. **The legacy `akerr_name_for_status(status, name)` set path cannot identify
|
## 6. Deprecate the two-argument name-registration path
|
||||||
its caller,** so it only checks that *some* reservation covers the status.
|
|
||||||
It is kept for migration. Consider deprecating it once consumers have moved
|
`akerr_name_for_status(status, name)` cannot identify its caller, so it can only
|
||||||
to `akerr_register_status_name()`.
|
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.
|
||||||
|
|
||||||
## Unrelated pre-existing issues
|
## Unrelated pre-existing issues
|
||||||
|
|
||||||
- The `AKERR_USE_STDLIB=OFF` build does not compile at all: `bool`, `PATH_MAX`
|
- 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.
|
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
|
The README's dependency list states what a replacement must provide, but the
|
||||||
provide, but the header still needs the includes untangled for that
|
header still needs its includes untangled for that configuration to work.
|
||||||
configuration to work.
|
- `CMakeLists.txt` sets `main_lib_dest` from `MY_LIBRARY_VERSION`, which is never
|
||||||
- `CMakeLists.txt` still sets `main_lib_dest` from `MY_LIBRARY_VERSION`, which is
|
defined and never read. Dead line.
|
||||||
never defined and never read. Dead line.
|
|
||||||
|
|||||||
Reference in New Issue
Block a user