Take libakerror 2.0.1 and drop the workaround it makes obsolete
Every libakgl test suite could report success while failing. libakerror's default unhandled-error handler ended in exit(errctx->status), an exit status is one byte wide, and libakgl's band starts at 256 -- so AKGL_ERR_SDL, the most common failure a library built on SDL can have, exited 0 and CTest recorded a pass. tests/character.c aborted at its second of four tests on a bad renderer and was green for months. 0.5.0 worked around that here with TEST_TRAP_UNHANDLED_ERRORS() in tests/testutil.h, and TODO.md ended the entry saying any consumer's suites have the same problem and it was worth raising upstream. It was. 2.0.1 fixes it at the source: akerr_exit() owns the mapping and the default handler calls it, so 0 exits 0, 1 through 255 exit themselves, and anything else exits AKERR_EXIT_STATUS_UNREPRESENTABLE (125). The trap and its 21 call sites are gone. Verified by putting the original failure back rather than by reading the release notes: a FAIL_BREAK(AKGL_ERR_SDL) in tests/character.c's main exits 125 and CTest reports a failure. A standalone consumer raising the same status unhandled exits 125 where it exited 0 before. tests/actor.c installs its own handler and called exit(errctx->status) from it, which is the same defect one layer up. It calls akerr_exit() now. 2.0.0 also makes the error pool and the status registry thread safe, which libakgl needs more than it knew: audio_stream_callback raises error contexts on SDL's audio thread. With an unlocked pool that callback and the main thread could scan AKERR_ARRAY_ERROR at the same time and be handed the same slot. The comment there says so. This is a hard dependency floor, not a preference. 2.0.0 moved __akerr_last_ignored to thread-local storage and made akerr_next_error() return a context that already holds its reference, and both expand at libakgl's call sites -- and at a consumer's, because akerror.h is part of libakgl's public interface. Mixing headers and libraries across that line double-counts every reference and never returns a pool slot. The soname moved to libakerror.so.2; include/akgl/error.h now also feature- tests AKERR_EXIT_STATUS_UNREPRESENTABLE, which is the narrowest probe for 2.0.1 since libakerror publishes no version macro. 0.7.0 for that reason: libakgl's own ABI is unchanged, but the one it re-exports through its headers is not. TODO.md records the pkg-config gap this makes sharper -- akgl.pc names no dependencies at all, so nothing tells a pkg-config consumer which libakerror it needs. Clean build, 26/26 ctest, memcheck clean, warning-clean at -Wall -Werror. libakgl.so.0.7 links libakerror.so.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8T5FAYXE8HEJqFLCYwNNc
This commit is contained in:
51
TODO.md
51
TODO.md
@@ -412,20 +412,25 @@ the symbol collision described in item 4 — and exited 0 because that status wa
|
||||
enough that `TODO.md` recorded the suite as passing and wondered which change
|
||||
had fixed it. Nothing had; it had stopped running.
|
||||
|
||||
**Fixed** in `tests/testutil.h`: `TEST_TRAP_UNHANDLED_ERRORS()` installs a
|
||||
handler that collapses any status a byte cannot carry onto 1, and every suite's
|
||||
`main()` calls it immediately after `akgl_error_init()`. `tests/error.c` calls
|
||||
it after its first test instead, because that suite has no `akgl_error_init()`
|
||||
in `main()` and libakerror's lazy `akerr_init()` would overwrite the handler.
|
||||
**Fixed twice.** 0.5.0 worked around it here, with a
|
||||
`TEST_TRAP_UNHANDLED_ERRORS()` in `tests/testutil.h` that installed a handler
|
||||
collapsing any status a byte cannot carry onto 1. Once installed, no other suite
|
||||
changed colour, so this was not masking anything beyond `character` — but it
|
||||
could have been at any time, and nothing would have said so.
|
||||
|
||||
Once installed, no other suite changed colour, so this was not masking anything
|
||||
beyond `character` — but it could have been at any time, and nothing would have
|
||||
said so.
|
||||
That entry ended "any consumer's test suites have this problem; that is worth
|
||||
raising upstream", and it was. **libakerror 2.0.1 fixes it at the source**:
|
||||
`akerr_exit()` owns the status-to-exit-code mapping and the default handler
|
||||
calls it, so 0 exits 0, 1 through 255 exit themselves, and anything else exits
|
||||
`AKERR_EXIT_STATUS_UNREPRESENTABLE` (125) rather than a low byte that is either
|
||||
a lie or a claim of success. There is no wider `exit()` to reach for —
|
||||
`_exit()`, `_Exit()`, `quick_exit()` and the raw `exit_group` syscall all
|
||||
truncate the same way, and even `waitid()` reports the truncated value.
|
||||
|
||||
Worth knowing: the fix belongs here rather than in libakerror only because
|
||||
`exit(status)` is a reasonable thing for a library to do when statuses fit in a
|
||||
byte. libakerror's own band does. Consumers' bands start at 256 by construction,
|
||||
so any consumer's test suites have this problem. That is worth raising upstream.
|
||||
The workaround is gone as of 0.6.0, along with its 21 call sites. Verified by
|
||||
putting the original failure back: a `FAIL_BREAK(AKGL_ERR_SDL)` in
|
||||
`tests/character.c`'s `main` now exits 125 and CTest records a failure, where it
|
||||
exited 0 and passed before.
|
||||
|
||||
## Coverage status
|
||||
|
||||
@@ -1884,6 +1889,28 @@ currently promises the opposite. `aksl_strncpy` already reports exactly that
|
||||
status when the bytes do not fit, so the change is to stop capping `n` at
|
||||
`size - 1` and let it raise.
|
||||
|
||||
## akgl.pc names no dependencies
|
||||
|
||||
`akgl.pc.in` carries `Libs: -L${libdir} -lakgl` and no `Requires:` or
|
||||
`Requires.private:` line, so a consumer that finds libakgl through pkg-config is
|
||||
told nothing about libakerror, libakstdlib, SDL3, SDL3_image, SDL3_mixer,
|
||||
SDL3_ttf or jansson.
|
||||
|
||||
That has always been wrong, and libakerror 2.x makes it sharper: `akerror.h` is
|
||||
part of libakgl's *public* interface -- `IGNORE` and the `FAIL_*` macros expand
|
||||
in the consumer's own translation units and reference `__akerr_last_ignored`,
|
||||
which is thread-local as of 2.0.0. A consumer that builds against whatever
|
||||
`akerror.h` happens to be on its include path and links whatever `libakerror.so`
|
||||
the loader finds can get a mismatch that pkg-config had every opportunity to
|
||||
prevent and did not mention.
|
||||
|
||||
The fix is a `Requires:` line for the libraries whose headers libakgl's headers
|
||||
include (akerror, akstdlib, SDL3) and `Requires.private:` for the rest, plus a
|
||||
version floor of `akerror >= 2.0.1`. Not done here because it changes what
|
||||
`pkg-config --libs akgl` emits for every existing consumer, and that wants to be
|
||||
its own commit with an install-tree test behind it. The CMake path is not
|
||||
affected -- `akglConfig.cmake` re-finds the targets.
|
||||
|
||||
## libakstdlib wrappers not yet adopted
|
||||
|
||||
`libakgl` links `libakstdlib` and uses ten of its wrappers. It calls the raw
|
||||
|
||||
Reference in New Issue
Block a user