From 0bb3a4d52c6e9e50e76405d73b0655faad6e1638 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Thu, 30 Jul 2026 13:22:50 -0400 Subject: [PATCH] TODO.md --- TODO.md | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..dc40fec --- /dev/null +++ b/TODO.md @@ -0,0 +1,143 @@ +# TODO + +Working notes for `libakerror`. Filed from the `akbasic` project, which is becoming the third +codebase to define custom status codes against this library (after `libakgl`; `libakstdlib` defines +none of its own). The items below are all facets of one question — **who owns the status-code number +space, and when is it decided** — but they are separable, and §1 can land on its own. + +Verification level is stated per item. Items marked **[read off source]** were established by reading +the library, its CMake, its `.pc.in` and its consumers; they have not been reproduced with a probe +program. Where a probe would settle a question, the item says so. + +--- + +## 1. The public header makes the name-table size part of the consumer ABI + +**[read off source — the corruption path is reasoned from the linker model, not reproduced]** + +`include/akerror.tmpl.h:51` declares: + +```c +extern char __AKERR_ERROR_NAMES[AKERR_MAX_ERR_VALUE+1][AKERR_MAX_ERROR_NAME_LENGTH]; +``` + +The object's real size comes from whatever `src/error.c` was compiled with. Every consumer's +*declaration* of it comes from whatever that consumer was compiled with. Nothing reconciles the two: +`akerror.pc.in` ships only `-I${includedir}`, `cmake/akerror.cmake.in` adds no compile definitions, +and a consumer that raises the value does so in its own build. `libakgl` raises it to 256 +(`CMakeLists.txt:44`, `target_compile_definitions(akerror PUBLIC AKERR_MAX_ERR_VALUE=256)`), which +propagates correctly *only* inside a build tree that vendored this library via `add_subdirectory`. + +So a consumer linking an installed `libakerror.so` can hold a different notion of the array's size +than the `.so` does, in either direction: + +- **Consumer declares smaller than the library was built with.** For a shared-library data symbol + referenced by an executable, the linker emits a copy relocation sized by the *executable's* + declaration and redirects the library's references to it. `akerr_init()` (`src/error.c:56`) then + memsets `(AKERR_MAX_ERR_VALUE+1) * 64` bytes using the library's larger value, into storage sized by + the smaller one. That is a BSS overflow at startup. **This is the reasoned worst case and is worth + confirming before acting on it** — build the library with `-DAKERR_MAX_ERR_VALUE=1024`, install it, + build a one-file consumer with the default, and run it under ASan. +- **Consumer declares larger.** No overflow, but `akerr_name_for_status()` bounds-checks against the + value *`error.c`* was compiled with (`src/error.c:121`), so the consumer's high codes never register + and every stack trace reports `"Unknown Error"`. + +The `#elif AKERR_MAX_ERR_VALUE < 256 / #error` guard at `include/akerror.tmpl.h:47` cannot catch +either case; it only rejects values below 256, never a disagreement between translation units. + +**The cheap fix is to delete line 51.** Nothing outside `src/error.c` references +`__AKERR_ERROR_NAMES` — the only other mentions in the tree are two explanatory comments +(`tests/err_maxval.c:6`, `tests/err_name_bounds.c:8`), neither of which touches the symbol. Making the +table static to `error.c` and reaching it exclusively through `akerr_name_for_status()` takes the size +out of the consumer-visible ABI entirely and turns `AKERR_MAX_ERR_VALUE` into a private build setting +of this library. That is worth doing regardless of which direction §2 goes, and it does not change any +documented API. + +A regression test would compile two objects with deliberately different `AKERR_MAX_ERR_VALUE` values +and link them; today that is silently accepted. + +## 2. There is no authority that allocates custom status codes, and it is decided too early + +**[read off source]** + +Codes are compile-time constants expressed as offsets from `AKERR_LAST_ERRNO_VALUE`. This library owns +`+1` … `+17` (`AKERR_NULLPOINTER` … `AKERR_BADEXC`; `+7` is unused, apparently a removed code). +`libakgl` continues the same sequence in its own header at `+18` … `+22` +(`include/akgl/error.h`). Nothing arbitrates that. The next library to want a code reads +`akgl/error.h`, picks `+23`, and hopes. + +Two consequences, both silent: + +- **Aliasing.** `__AKERR_ERROR_NAMES` is a single global keyed by the integer, so two owners choosing + the same offset means last-registration-wins for the name, and — worse — `HANDLE(e, FOO_ERR_X)` + catching `BAR_ERR_Y`, since `HANDLE` compares the same integer. `akbasic` is intended to link + *into* `libakgl` as a scripting engine, so co-resident custom codes are the expected configuration, + not a corner case. +- **Drift.** Because the codes are relative to `AKERR_LAST_ERRNO_VALUE`, which `scripts/generrno.sh` + derives from the host's `errno --list`, adding an 18th `AKERR_*` code here silently aliases + `AKGL_ERR_SDL`, and a libc that grows an errno moves every downstream code. + +There is also a documentation/practice split worth resolving either way: `README.md:67` says values up +to 255 are reserved and consumers must start at 256, but `libakgl`'s codes land around 151 on glibc — +inside the reserved band — and the `#error` guard's 256 floor implies the README's rule while nothing +enforces it. Note too that `AKGL_ERR_SDL` (`+18`) already exceeds this library's *default* +`AKERR_MAX_ERR_VALUE` (`+17`), which is why `libakgl` must raise it; any `libakgl` consumer that +doesn't inherit that definition loses all of akgl's error names. + +### On the shape of a fix — options, not a prescription + +A static registry of reserved bases in this header (`AKERR_ERR_BASE_AKGL`, …) was considered and +rejected: it would require the library to enumerate every application that will ever link it. The +allocation decision belongs to the consumer; what's missing is a way to *declare* it and have +conflicts detected. Some directions, roughly in order of invasiveness — the trade-offs are laid out +so the choice can be made here, not imposed from outside. + +**(a) Make the name table sparse, and the number space unbounded.** The dense +`[AKERR_MAX_ERR_VALUE+1][64]` array is the root of most of this: indexing by status forces an upper +bound, the bound has to be visible to size the array, and a consumer-visible bound is what creates §1. +Replacing it with a fixed-capacity table of `{ int status; char name[64]; }` searched by status (linear +scan, or open addressing if the constant grows) keeps the no-dynamic-allocation rule, keeps codes as +compile-time constants so `case` labels in `HANDLE` still work, keeps `akerr_name_for_status()`'s +signature, and makes *any* `int` a legal status. `AKERR_MAX_ERR_VALUE` then becomes a private capacity +constant, or disappears. This is the smallest change that dissolves the ABI problem, and it composes +with everything below. + +**(b) Let consumers declare their range at init, and detect overlap loudly.** Something like +`akerr_reserve_status_range(base, count, "akbasic")`, called from each library's init, returning an +error context on overlap with an already-reserved range. The library predicts nobody; it just records +who claimed what and refuses a double claim. This converts the silent aliasing in §2 into a startup +failure with a name in the message, which is the actual goal. Cheap, additive, and testable. Pairs +naturally with (a). + +**(c) Allocate codes at runtime.** `int akerr_register_status(const char *name)` handing out fresh +integers, with consumers storing them in globals initialised at startup. This is the fully dynamic +end, and it has a specific blocker worth knowing before it gets far: **the control-flow macros are +built on `switch`, and `HANDLE`/`HANDLE_GROUP` expand to `case` labels, which C requires to be integer +constant expressions.** A runtime-allocated code cannot appear in a `case`. Adopting this means +rewriting `PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` as an `if`/`else if` ladder. +That is not unthinkable — and it would incidentally let `HANDLE` match on things a `case` can't, like +a range or a predicate — but it touches the most load-bearing code in the library and every consumer's +error handling at once. Note that it would *not* by itself fix the "don't use `CATCH` or +`FAIL_*_BREAK` inside a loop" hazard (`README.md:237`): that comes from exiting via `break`, not from +`switch` specifically, and would need its own treatment. + +**(d) Namespace the status instead of partitioning the integers.** Carry `(domain, code)` in +`akerr_ErrorContext` so each library numbers from 1 in its own domain and collision becomes +structurally impossible. Conceptually the cleanest and the most invasive: it changes the context +struct, every `HANDLE` call site across three repos, and the stack-trace format. Probably only worth +it if the macro layer is being reworked for (c) anyway. + +Whichever way this goes, a migration note matters: `libakgl` currently depends on `+18` … `+22` being +valid *and* on `AKERR_MAX_ERR_VALUE=256` reaching its own translation units. Any change here wants a +coordinated bump of that submodule, or a compatibility window where the old offsets keep working. + +## 3. `tests/err_maxval.c` guards the invariant that is going away + +**[read off source]** + +`tests/err_maxval.c` parses the generated `akerror.h` to assert `AKERR_MAX_ERR_VALUE` covers the +highest `AKERR_*` offset — a good guard against the historical bug it documents (max at `+15` while +`AKERR_BADEXC` is `+17`). If §1 or §2(a) lands, this test's premise changes: with the table private or +sparse, the thing worth asserting is no longer "the ceiling covers our own codes" but "a consumer code +outside our range still registers and retrieves". Worth rewriting alongside, not deleting — the +regression it was written for should stay covered in whatever form the ceiling takes.