144 lines
9.5 KiB
Markdown
144 lines
9.5 KiB
Markdown
|
|
# 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.
|