Version at 0.2.0: complete the wishlist, document it, gate the docs
Closes what was left of TODO.md sections 1, 2 and 3, and rewrites that
file to hold outstanding items only.
The API break gets a minor bump, because pre-1.0 the soname carries
MAJOR.MINOR and 0.1 and 0.2 are therefore different ABIs. Five
signatures changed and the ato* contract with them; UPGRADING.md is new
and lists every one, with the before/after for the cases the compiler
cannot warn about.
Section 3.1 is finished: reallocarray with the multiplication checked,
aligned_alloc and posix_memalign, asprintf/vasprintf, scanf/vscanf.
Four functions on that list are deliberately absent rather than missing
-- sprintf, strtok, setbuf and perror -- and TODO.md now says which and
why, so nobody adds them thinking they were forgotten.
Section 1.9, the cross-cutting tests:
tests/test_pool.c drives every failure path AKERR_MAX_ARRAY_ERROR
+ 10 times and checks the pool after each round,
because a wrapper that leaks a slot fails a
hundred calls later in unrelated code. It also
asserts that each error names the function and
file it was raised from, which is what catches a
FAIL that migrates into a helper during a
refactor: status right, message right, origin
quietly lying.
tests/negative/ two sources that must FAIL to compile, built with
-Werror and registered WILL_FAIL. AKERR_NOIGNORE
and the format attributes are enforced by the
compiler and by nothing else; drop either and
every ordinary test still passes.
Thread safety is answered rather than tested: the library is not
thread-safe and cannot be made so from here, because libakerror's error
pool is an unlocked process-global array. README.md says so plainly and
TODO.md carries it as the item blocking any future pthread wrappers.
Doxygen is configured and gated. All 147 public functions have @brief,
a @param each, @throws per status and @return; EXTRACT_ALL is off and
WARN_NO_PARAMDOC on, so `cmake --build build --target docs` fails on an
undocumented entity. It ran to 0 warnings. The Doxyfile carries no
version -- cmake/RunDoxygen.cmake feeds PROJECT_NUMBER in from
project(), so that stays the one place a version is written.
CI now builds against the submodule it pins instead of also installing
libakerror@main and never linking it, adds -Werror, and gains a
sanitizer job. The pre-push hook matches, and runs the docs check too.
Coverage: 99.5% of lines (1643/1651), 100% of functions (147/147). The
eight uncovered lines are each uncovered on purpose and TODO.md says
which and why.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
838
TODO.md
838
TODO.md
@@ -1,555 +1,178 @@
|
||||
# TODO
|
||||
|
||||
Working notes for `libakstdlib` — the akerror-wrapped libc surface.
|
||||
Working notes for `libakstdlib`. **Outstanding items only** — anything fixed comes
|
||||
out of this file and goes into `UPGRADING.md`, the tests, or a comment beside the
|
||||
code, whichever is the right place to be reminded of it.
|
||||
|
||||
Scope of the current library (`include/akstdlib.h`, `src/stdlib.c`): 16 libc wrappers
|
||||
(`fopen`, `fread`, `fwrite`, `fclose`, `malloc`, `free`, `memset`, `memcpy`, `printf`,
|
||||
`fprintf`, `sprintf`, `atoi`, `atol`, `atoll`, `atof`, `realpath`), one hash helper
|
||||
(`aksl_strhash_djb2`), a doubly-linked list (`append`/`pop`/`iterate`) and a binary tree
|
||||
iterator (`aksl_tree_iterate`).
|
||||
Ordered by blast radius: what blocks other work first, what is merely wrong
|
||||
second, what is missing last.
|
||||
|
||||
Items marked **[CONFIRMED]** were reproduced by building the library and running a probe
|
||||
program against it, not just read off the source.
|
||||
## Where the library stands
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| Wrapped | 147 public functions across `src/stdlib.c`, `src/string.c`, `src/stream.c`, `src/collections.c` |
|
||||
| Tests | 17 CTest binaries plus 2 negative-compile entries, green under the default and sanitizer builds |
|
||||
| Line coverage | 99.5% (1643/1651) |
|
||||
| Function coverage | 100% (147/147) |
|
||||
| Doxygen | 100%, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return |
|
||||
|
||||
The six confirmed defects that used to head this file are fixed and
|
||||
`AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a
|
||||
result, is in `UPGRADING.md`.
|
||||
|
||||
---
|
||||
|
||||
## 1. Unit tests that should be written
|
||||
## 1. Blocked on libakerror
|
||||
|
||||
### 1.0 Test-harness prerequisites — **DONE** (branch `feature/test-harness`)
|
||||
These are not fixable from inside this repository. Each one currently costs
|
||||
something here, and the cost is what makes them worth carrying.
|
||||
|
||||
`ctest` is green: 5/5, no *Not Run* entries. Everything in §1.1–1.9 can now be written
|
||||
against `tests/aksl_capture.h` and added to the `AKSL_TESTS` list.
|
||||
### 1.1 The error pool is a process-global array with no locking
|
||||
|
||||
- [x] **The current tests cannot fail.** `tests/test_linkedlist.c` asserted nothing at all;
|
||||
it printed node names and returned `0` unconditionally, so any of the list bugs in
|
||||
§2.1 would pass it. Rewritten as 15 assertion-based cases covering the append, iterate
|
||||
and pop behaviour that is correct today.
|
||||
- [x] **`tests/test_tree.c` was red.** `ctest` reported `tree (Failed)`, exit 136, because
|
||||
`parms.steps` was never reset between the three searches, so the second assertion
|
||||
compared an accumulated `14` against `7`. Each search now builds its own tree and
|
||||
params. The file records that its step counts do not actually distinguish the three
|
||||
traversal orders — real order assertions remain §1.8.
|
||||
- [x] **Adopt libakerror's test conventions.** `tests/aksl_capture.h` provides `AKSL_CHECK()`
|
||||
(an `NDEBUG`-proof assert), `AKSL_CHECK_STATUS()`/`AKSL_CHECK_OK()` which run an
|
||||
akerror-returning call, assert on its status and release the context,
|
||||
`AKSL_CHECK_MSG_CONTAINS()`, a capturing `akerr_log_method`, `aksl_slots_in_use()` for
|
||||
pool-leak checks, and an `AKSL_RUN()` driver that additionally fails any test which
|
||||
leaks an error-pool slot.
|
||||
- [x] **One test source per behaviour, registered with CTest via a list variable.**
|
||||
`tests/test_<name>.c` → executable `test_<name>` → CTest test `<name>`, driven by
|
||||
`AKSL_TESTS` in `CMakeLists.txt`.
|
||||
- [x] **Add a `WILL_FAIL` category.** Two lists: `AKSL_WILL_FAIL_TESTS` for tests that abort
|
||||
by design (empty for now) and `AKSL_KNOWN_FAILING_TESTS` for the confirmed defects in
|
||||
§2.1 — see the note at the head of §2.1.
|
||||
- [x] **Fix the four permanently-failing submodule tests.** `deps/libakerror` is added
|
||||
`EXCLUDE_FROM_ALL`, so akerror's `add_test` entries registered but its test binaries
|
||||
were never built, and `ctest` reported `err_catch`, `err_cleanup`, `err_trace`,
|
||||
`err_improper_closure` as *Not Run* → failed on every run. Neither the pinned commit
|
||||
nor upstream `main` guards that registration, CMake has no way to un-register a test,
|
||||
and `set_tests_properties` cannot reach across directory scopes — so `add_test` and
|
||||
`set_tests_properties` are shadowed for the duration of the `add_subdirectory` call.
|
||||
The dependency has its own CI; this suite now contains only this project's tests.
|
||||
- [x] **Wire in a memory-error build.** `cmake -S . -B build-asan -DAKSL_SANITIZE=ON` adds
|
||||
`-fsanitize=address,undefined -fno-sanitize-recover=all` to the library, the tests and
|
||||
the vendored libakerror. Currently clean, and it is the intended way to test the items
|
||||
in §2 that only misbehave under instrumentation (uninitialised `%s` in
|
||||
`aksl_realpath`, unbounded `vsprintf`, missing `va_end`).
|
||||
- [x] **Port `scripts/mutation_test.py` from libakerror.** Retargeted at `src/stdlib.c` and
|
||||
`include/akstdlib.h` (178 mutants) and exposed as
|
||||
`cmake --build build --target mutation`. CI runs the narrower `src/stdlib.c` set (173
|
||||
mutants) as its own `mutation_test` job with `--threshold 80` and a JUnit report.
|
||||
**Current score: 89.6%** — 155 killed, 18 survived. It was 46.8% (81 killed, 92
|
||||
survived) before §1.2–1.6 were written; the remaining survivors are:
|
||||
**This is what makes the library single-threaded**, and it is the largest open
|
||||
item by some distance.
|
||||
|
||||
| surviving mutants | where | why |
|
||||
|---|---|---|
|
||||
| 5 | deleted statements whose absence nothing observes (`free(ptr)`, `obj->next = NULL`, three `SUCCEED_RETURN`s) | needs an assertion on the side effect, not on the status |
|
||||
| 4 | `aksl_list_append`'s cycle/tail walk | the function is broken here (§2.1.1); its known-failing test cannot pin the internals yet |
|
||||
| 4 | `lalloc`/`lfree` defaulting in `aksl_tree_iterate` | dead parameters (§2.2.8) — defaulted, then never called |
|
||||
| 3 | `*count == -1` in the `printf` family | `*count` on the error path is itself a contract gap (§1.3) |
|
||||
| 2 | `feof` in `aksl_fwrite`, `break` after the BFS `FAIL_RETURN` | unreachable in practice |
|
||||
`AKERR_ARRAY_ERROR` is a fixed array in `deps/libakerror/src/error.c`, handed out
|
||||
by `akerr_next_error()` with no synchronisation of any kind. Every entry point in
|
||||
this library takes a slot from it on any failure path, so two threads raising
|
||||
errors concurrently can be handed the same slot and will corrupt each other's
|
||||
message, status and stack trace.
|
||||
|
||||
The threshold is a regression ratchet, not a quality bar; raise it as those survivors
|
||||
become assertions. Each one is a concrete missing test — `mutation-junit.xml` lists
|
||||
them with `file:line` and the exact edit.
|
||||
- [x] **Cap every test with a CTest `TIMEOUT`.** Found while measuring the above: a mutant
|
||||
that deletes `fast = fast->next->next` turns `aksl_list_iterate` into an infinite
|
||||
loop, so `ctest` waited forever, the mutation harness killed `ctest` at its own
|
||||
120 s timeout, and the orphaned test binary kept spinning at 100% CPU while holding
|
||||
the pipe open — the run wedged and had to be killed by hand. `TIMEOUT 30` on every
|
||||
test makes `ctest` reap its own child, so those mutants are now killed in 30 s and
|
||||
leave nothing behind. It also guards the real suite against the same class of bug.
|
||||
- [x] **CI could not have run any of this.** `actions/checkout@v4` was not fetching
|
||||
submodules, so `add_subdirectory(deps/libakerror)` had nothing to descend into and the
|
||||
configure step failed before ever reaching the tests. Added `submodules: recursive`,
|
||||
and swapped `cmake --build build --target test` for `ctest --output-on-failure` so a
|
||||
failure is diagnosable from the job log. The deeper problem — CI installing
|
||||
`libakerror@main` while the build actually compiles the pinned submodule — is §2.3.
|
||||
**Consequence.** `README.md` says plainly that the library is not thread-safe.
|
||||
That is honest, and it is also a hard ceiling: §4's `pthread_*` and socket
|
||||
wrappers cannot be written until this is resolved, because a threading API
|
||||
nobody can call from a thread is not an API.
|
||||
|
||||
### 1.1 Memory wrappers
|
||||
**What closing it would touch.** libakerror's pool — either a mutex around
|
||||
`akerr_next_error`/`akerr_release_error`, or thread-local slot arrays, which
|
||||
would suit the bounded-preallocation style better and cost nothing on the
|
||||
single-threaded path. Then a TSan job in `.gitea/workflows/ci.yaml` and a
|
||||
concurrent smoke test here, and the warning in `README.md` comes out.
|
||||
|
||||
- [x] `aksl_malloc` — happy path returns non-NULL and writes through `dst`.
|
||||
- [x] `aksl_malloc(n, NULL)` → `AKERR_NULLPOINTER`, message content asserted.
|
||||
- [ ] `aksl_malloc(0, &p)` — pin down the contract. Today the result depends on whether the
|
||||
platform's `malloc(0)` returns NULL; if it does, the wrapper reports `errno`, which may
|
||||
be `0`. See §2.2.1.
|
||||
- [ ] `aksl_malloc(SIZE_MAX, &p)` → allocation failure surfaces `ENOMEM`, and `*dst` is left
|
||||
NULL rather than garbage.
|
||||
- [x] `aksl_free(NULL)` → `AKERR_NULLPOINTER` (assert the documented behaviour, since
|
||||
`free(NULL)` is legal C and callers will be surprised).
|
||||
- [x] `aksl_free` happy path, and a malloc→free round trip under ASan.
|
||||
- [x] `aksl_memset` — happy path fills the buffer; `n == 0` is a no-op; `s == NULL` →
|
||||
`AKERR_NULLPOINTER`.
|
||||
- [x] `aksl_memcpy` — happy path copies; `d == NULL` and `s == NULL` each →
|
||||
`AKERR_NULLPOINTER`; `n == 0` with valid pointers succeeds.
|
||||
- [ ] `aksl_memcpy` with overlapping regions — decide and test whether this is rejected
|
||||
(`AKERR_VALUE`) or documented as UB like `memcpy`.
|
||||
### 1.2 `IGNORE()` logs a context and never releases it
|
||||
|
||||
### 1.2 File I/O
|
||||
`deps/libakerror/include/akerror.tmpl.h:308`. The macro assigns the context to
|
||||
`__akerr_last_ignored`, logs it, and stops. The slot is never returned to the
|
||||
pool, so every `IGNORE()` on a failing call leaks one — and after
|
||||
`AKERR_MAX_ARRAY_ERROR` of them the pool is exhausted and `ENSURE_ERROR_READY`
|
||||
calls `exit(1)`.
|
||||
|
||||
Covered by `tests/test_stream.c`.
|
||||
**Consequence here.** `aksl_tree_iterate`'s `CLEANUP` block (`src/stdlib.c`)
|
||||
cannot use `IGNORE()` to drop a queue-drain failure and open-codes the
|
||||
log-then-release by hand instead, with a comment saying why. It is four lines
|
||||
that should be one.
|
||||
|
||||
- [x] `aksl_fopen` happy path on a temp file; `*fp` is written.
|
||||
- [x] `aksl_fopen("/nonexistent/path", "r", &fp)` → `ENOENT` propagated as the status, with
|
||||
the pathname in the message.
|
||||
- [ ] `aksl_fopen` on a mode-denied path (e.g. `/proc/1/mem`, or a `chmod 000` temp file) →
|
||||
`EACCES`.
|
||||
- [x] `aksl_fopen(path, mode, NULL)` → `AKERR_NULLPOINTER`.
|
||||
- [ ] `aksl_fopen(NULL, "r", &fp)` and `aksl_fopen(path, NULL, &fp)` → currently unchecked,
|
||||
see §2.2.2. Test once the guards exist.
|
||||
- [x] `aksl_fread` full read; short read at EOF → `AKERR_EOF`; read from a write-only stream
|
||||
→ `AKERR_IO`; `fp == NULL` → `AKERR_NULLPOINTER`. `ptr == NULL` is still unchecked and
|
||||
untested (see §2.2.3).
|
||||
- [ ] `aksl_fread` **partial read that is neither EOF nor error** — assert whatever the fixed
|
||||
contract is; today this silently returns success and the caller cannot tell how many
|
||||
members were read.
|
||||
- [x] `aksl_fwrite` happy path; write to a read-only stream → `AKERR_IO`; `fp == NULL` →
|
||||
`AKERR_NULLPOINTER`. The full-device (`/dev/full`) case is still open.
|
||||
- [x] `aksl_fclose` happy path; `NULL` → `AKERR_NULLPOINTER`. Double-close is still
|
||||
undecided, so it is neither caught nor tested.
|
||||
- [ ] `aksl_fclose` on a stream whose buffered flush fails (`/dev/full`) → non-zero `fclose`
|
||||
surfaces `errno`.
|
||||
- [x] Round-trip test: `fopen` → `fwrite` → `fclose` → `fopen` → `fread` → compare bytes.
|
||||
**What closing it would touch.** One `RELEASE_ERROR` in the macro; then delete
|
||||
the workaround here.
|
||||
|
||||
### 1.3 Formatted output
|
||||
### 1.3 libakerror does not namespace its `coverage` target when embedded
|
||||
|
||||
Covered by `tests/test_format.c`.
|
||||
`deps/libakerror/CMakeLists.txt:172` versus `:189` — it namespaces `mutation` and
|
||||
not `coverage`.
|
||||
|
||||
- [x] `aksl_printf` / `aksl_fprintf` / `aksl_sprintf` happy paths, asserting both the byte
|
||||
count written through `count` and the produced text. (`aksl_printf` is checked by
|
||||
pointing `stdout` at a temp file for the duration of the call.)
|
||||
- [x] Each of the three with every pointer argument NULL in turn → `AKERR_NULLPOINTER`.
|
||||
- [x] `aksl_fprintf` to a read-only stream → the `errno` it saw (`EBADF`) with "Short write"
|
||||
in the message. `*count` is still left holding `-1`; the test asserts the status only,
|
||||
so it will not have to change when that is fixed.
|
||||
- [ ] `aksl_sprintf` with a format that overflows the destination — currently unbounded
|
||||
(§2.2.4). Test the `aksl_snprintf` replacement once it exists.
|
||||
- [x] Regression test for the missing `va_end` (§2.1.4) — 512 variadic calls in a loop, which
|
||||
the sanitizer build also runs.
|
||||
- [ ] Format-string/arg mismatch is caught at compile time once
|
||||
`__attribute__((format(printf, ...)))` is added (§2.2.5) — a negative compile test.
|
||||
**Consequence here.** A `-DAKSL_COVERAGE=ON` top-level build fails to configure
|
||||
at all: *"another target with the same name already exists"*. `CMakeLists.txt`
|
||||
shadows `add_custom_target` for the duration of the `add_subdirectory()` call and
|
||||
renames the dependency's to `akerror_coverage`.
|
||||
|
||||
### 1.4 String → number
|
||||
**What closing it would touch.** Apply the same
|
||||
`CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR` test upstream that the
|
||||
`mutation` target already has; then delete the shadow here, which sits directly
|
||||
above the `add_test` shadow and shares its comment.
|
||||
|
||||
Happy paths and NULL guards are covered by `tests/test_convert.c`; the strict-conversion
|
||||
contract is asserted by `tests/test_convert_strict.c`, which is registered as a known
|
||||
failure until §2.1.5 is fixed.
|
||||
### 1.4 libakerror installs no `akerrorConfigVersion.cmake`
|
||||
|
||||
- [x] `aksl_atoi` / `atol` / `atoll` / `atof` happy paths, including negative values and
|
||||
leading whitespace.
|
||||
- [x] NULL `nptr` and NULL `dest` for each → `AKERR_NULLPOINTER`.
|
||||
- [x] **Non-numeric input** (`"not a number"`) — **[CONFIRMED]** currently returns *success*
|
||||
with `*dest == 0`. `AKERR_VALUE` is asserted by the known-failing test.
|
||||
- [x] **Overflow** (`"99999999999999999999"`) — **[CONFIRMED]** currently returns success with
|
||||
a garbage value (`-1` on this box). `ERANGE` is asserted by the known-failing test.
|
||||
- [x] Empty string, `" "` and `"12abc"` (trailing junk) → `AKERR_VALUE`, in the known-failing
|
||||
test. `"0x10"` and `"inf"`/`"nan"` for `atof` are still open.
|
||||
- [ ] `LONG_MIN`/`LONG_MAX`/`LLONG_MIN`/`LLONG_MAX` boundary strings round-trip exactly.
|
||||
**Consequence here.** `cmake/akstdlib.cmake.in` has to call
|
||||
`find_dependency(akerror)` with no version, because a request for one would be
|
||||
refused for want of a version file no matter what is installed. The 1.0.0 floor
|
||||
therefore rests on `akstdlib.pc`'s `Requires:` and the `#error` guard in
|
||||
`akstdlib.h`, neither of which covers a `find_package` consumer.
|
||||
|
||||
### 1.5 `aksl_realpath`
|
||||
|
||||
Covered by `tests/test_path.c`. Every failure case there passes a zeroed `resolved_path`,
|
||||
because the wrapper's own error path formats that buffer with `%s` (§2.1.6).
|
||||
|
||||
- [x] Happy path on an existing file and on a symlink chain; result matches `realpath(3)`.
|
||||
- [x] Non-existent path → `ENOENT`.
|
||||
- [x] A path component that is not a directory → `ENOTDIR`.
|
||||
- [ ] Symlink loop → `ELOOP`.
|
||||
- [x] `path == NULL` → `AKERR_NULLPOINTER`.
|
||||
- [ ] `resolved_path == NULL` — currently unchecked and leaks (§2.1.6). Test once fixed.
|
||||
- [ ] A failure case where `resolved_path` is an *uninitialised* buffer — this is the crash
|
||||
case in §2.1.6; run it under ASan/MSan.
|
||||
|
||||
### 1.6 `aksl_strhash_djb2`
|
||||
|
||||
Covered by `tests/test_strhash.c`.
|
||||
|
||||
- [x] Known-answer vectors: `djb2("")` == 5381; a handful of fixed strings with their
|
||||
pre-computed 32-bit values.
|
||||
- [x] `len == 0` returns 5381 regardless of `str` contents.
|
||||
- [x] NULL `str` and NULL `hashval` → `AKERR_NULLPOINTER`.
|
||||
- [ ] **High-bit bytes** (`"\xff\xfe"`) — pins down the sign-extension bug in §2.2.6; the
|
||||
expected value must be the `unsigned char` one (5868578; the wrapper returns the
|
||||
sign-extended 5859874 today). Every vector in the test file is 7-bit ASCII precisely so
|
||||
that it says nothing about this case either way.
|
||||
- [x] Embedded NUL bytes are hashed (the function is length-driven, not NUL-driven).
|
||||
- [x] Same input → same output across two calls (no hidden state).
|
||||
|
||||
### 1.7 Linked list
|
||||
|
||||
- [ ] **`aksl_list_append` builds a correct chain of N nodes** — **[CONFIRMED BROKEN]**, see
|
||||
§2.1.1. Appending `n1..n4` to `n0` yields the chain `n0 -> n4`; `n1`, `n2`, `n3` are
|
||||
silently dropped. This is the single most important test to add.
|
||||
- [ ] `append` sets `obj->prev` to the real tail and `obj->next` to NULL.
|
||||
- [ ] `append` onto an empty (single, zeroed) node.
|
||||
- [ ] `append` NULL list / NULL obj → `AKERR_NULLPOINTER`.
|
||||
- [ ] `append` onto a list containing a cycle → `AKERR_CIRCULAR_REFERENCE`, for a self-loop,
|
||||
a 2-node cycle, and a cycle that does not include the head.
|
||||
- [ ] `append` a node that is already in the list (aliasing) — define and test the contract.
|
||||
- [ ] **`aksl_list_iterate` visits every node exactly once, starting at the head** —
|
||||
**[CONFIRMED BROKEN]**, see §2.1.2: it starts iterating from the *midpoint* left behind
|
||||
by the cycle detector, so the head is never visited. Assert both the visit count and
|
||||
the visit order.
|
||||
- [ ] `iterate` over a single-node list visits exactly one node.
|
||||
- [ ] `iterate` NULL list / NULL iter → `AKERR_NULLPOINTER`.
|
||||
- [ ] `iterate` over a cyclic list → `AKERR_CIRCULAR_REFERENCE` (self-loop, 2-node,
|
||||
tail-to-middle).
|
||||
- [ ] `iterate` where the callback raises `AKERR_ITERATOR_BREAK` → iteration stops at that
|
||||
node, the wrapper returns success, and the visit count proves the early exit.
|
||||
- [ ] `iterate` where the callback raises some *other* error → that error propagates out
|
||||
unchanged, with the callback's message intact.
|
||||
- [ ] `aksl_list_pop` on a middle node relinks `prev`/`next` correctly and clears the popped
|
||||
node's pointers.
|
||||
- [ ] `pop` on the head node, on the tail node, and on a single-node list.
|
||||
- [ ] `pop(NULL)` → `AKERR_NULLPOINTER`.
|
||||
- [ ] `pop` then `iterate` — the list is still traversable and the popped node is gone.
|
||||
- [ ] Pool-accounting test: after a long sequence of list operations including failures,
|
||||
`akerr_slots_in_use() == 0`.
|
||||
|
||||
### 1.8 Tree
|
||||
|
||||
- [ ] Visit **order** assertions, not just counts, for `DFS_PREORDER`, `DFS_INORDER` and
|
||||
`DFS_POSTORDER` over the 7-node tree — record the visited node pointers into an array
|
||||
and compare against the expected sequence. The current test only counts steps, which
|
||||
cannot distinguish the three orders (all three visit 7 nodes).
|
||||
- [ ] **`AKERR_ITERATOR_BREAK` aborts the whole traversal** — **[CONFIRMED BROKEN]**, see
|
||||
§2.1.3. The pre-order case exists as `tests/test_tree_iterate_break.c`: pre-order
|
||||
visits 0, 1, 3, so breaking at `tree[3]` must stop the walk at **3** visits, and today
|
||||
it runs on to all 7. Add the equivalent for in-order (3, 1, 4, 0, 5, 2, 6 → breaking at
|
||||
`tree[4]` gives 3) and post-order (3, 4, 1, 5, 6, 2, 0 → breaking at `tree[5]` gives 4).
|
||||
- [x] `AKSL_TREE_SEARCH_BFS` and `AKSL_TREE_SEARCH_BFS_RIGHT` → `AKERR_NOT_IMPLEMENTED`
|
||||
today, asserted in `tests/test_tree.c` along with the callback never being called;
|
||||
replace with real order assertions once implemented (§3 / §2.2.9).
|
||||
- [x] `aksl_tree_iterate` NULL `root` / NULL `iter` → `AKERR_NULLPOINTER`, and a callback error
|
||||
that is *not* `AKERR_ITERATOR_BREAK` propagates out to the caller.
|
||||
- [ ] **Unknown `searchmode`** (e.g. `99`) — **[CONFIRMED]** currently returns *success*
|
||||
having visited nothing. Should be `AKERR_VALUE`.
|
||||
- [ ] **`AKSL_TREE_SEARCH_VISIT`** (defined in the header, `5`) — **[CONFIRMED]** falls into
|
||||
the same silent-success hole. Either implement it or reject it.
|
||||
- [ ] `root == NULL` / `iter == NULL` → `AKERR_NULLPOINTER`.
|
||||
- [ ] Single-node tree (no children) visits exactly once, in all three orders.
|
||||
- [ ] Left-only and right-only degenerate chains.
|
||||
- [ ] Callback raising a non-`ITERATOR_BREAK` error propagates out of the recursion with the
|
||||
original status and message.
|
||||
- [ ] Custom `lalloc`/`lfree` are actually invoked — **currently they are stored and never
|
||||
called** (§2.2.8), so this test will fail until BFS lands.
|
||||
- [ ] Deep/degenerate tree (e.g. 100k-node left chain) — documents the recursion-depth limit
|
||||
(§2.2.7).
|
||||
- [ ] A tree containing a cycle (child pointing back at an ancestor) — currently infinite
|
||||
recursion; test for `AKERR_CIRCULAR_REFERENCE` once guarded.
|
||||
|
||||
### 1.9 Cross-cutting
|
||||
|
||||
- [ ] **Error-pool accounting**: for *every* wrapper, a test that drives the failure path
|
||||
`AKERR_MAX_ARRAY_ERROR + 10` times and asserts the pool does not leak
|
||||
(`akerr_slots_in_use() == 0` after each handled error).
|
||||
- [ ] **Stack-trace content**: assert that the file/function/line recorded by each wrapper's
|
||||
`FAIL` points at `src/stdlib.c` and the right function name.
|
||||
- [ ] **`AKERR_NOIGNORE` is effective**: a negative compile test where a wrapper's return is
|
||||
discarded and `-Werror=unused-result` fires.
|
||||
- [ ] **Thread safety**: libakerror's `AKERR_ARRAY_ERROR` is a process-global array with no
|
||||
locking. If `libakstdlib` is meant to be callable from threads, add a concurrent
|
||||
smoke test under TSan — and if it is not, say so in the README.
|
||||
**What closing it would touch.** One `write_basic_package_version_file()` call
|
||||
upstream — libakstdlib already does this correctly and can be copied — then add
|
||||
the `1.0.0` floor to the `find_dependency` here.
|
||||
|
||||
---
|
||||
|
||||
## 2. Corner cases in existing functionality that should be resolved
|
||||
## 2. Known gaps in what is already wrapped
|
||||
|
||||
### 2.1 Confirmed defects (reproduced against the built library)
|
||||
### 2.1 A short transfer with neither EOF nor a stream error is untested
|
||||
|
||||
Four of these now have a failing test apiece, registered in `AKSL_KNOWN_FAILING_TESTS`
|
||||
and therefore marked `WILL_FAIL` so the suite stays green while the gap stays visible:
|
||||
`tests/test_list_append_chain.c`, `tests/test_list_iterate_head.c`,
|
||||
`tests/test_tree_iterate_break.c` and `tests/test_convert_strict.c`. When one of these
|
||||
defects is fixed, CTest reports that test as failed with *unexpectedly passed* — that is the
|
||||
cue to move it into `AKSL_TESTS`.
|
||||
`src/stdlib.c`, the `FAIL_RETURN(e, AKERR_IO, "short read: ...")` in `aksl_fread`
|
||||
and its counterpart in `aksl_fwrite`. Two of the eight uncovered lines in the
|
||||
whole library.
|
||||
|
||||
1. **`aksl_list_append` does not find the tail — it silently truncates the list.**
|
||||
`src/stdlib.c:194`. The function conflates Floyd cycle detection with tail-finding:
|
||||
`tail` is set to `slow` *before* `slow` advances, so it tracks the node *behind the
|
||||
midpoint*, not the tail. Appending `n1`, `n2`, `n3`, `n4` to `n0` produces the chain
|
||||
`n0 -> n4`; `n1`–`n3` are unlinked and lost. The fix is to keep the cycle check but walk
|
||||
a separate cursor to the real tail (`while (tail->next) tail = tail->next;`), or run
|
||||
Floyd first and then walk to the end.
|
||||
The standard permits a short transfer with neither indicator set, so the branch
|
||||
is correct to have. Every way of actually producing one on Linux sets `feof` or
|
||||
`ferror` first, so nothing in the suite reaches it.
|
||||
|
||||
2. **`aksl_list_iterate` skips the first half of the list.** `src/stdlib.c:324`. After the
|
||||
cycle-detection loop, `slow` is left at the list midpoint, and the visiting loop then
|
||||
starts from `slow` instead of from `list`. The head node is never passed to the callback.
|
||||
Fix: iterate from `list`, not from `slow`.
|
||||
**Consequence.** Low: the code is a handful of lines and reviewed, but it is the
|
||||
only error path in the library that has never executed.
|
||||
|
||||
3. **`AKERR_ITERATOR_BREAK` does not stop a tree traversal.** `src/stdlib.c:251`. The
|
||||
recursive frame in which the callback raises the break handles it in its own
|
||||
`PROCESS`/`HANDLE(e, AKERR_ITERATOR_BREAK)` block and returns *success*; the parent's
|
||||
`PASS` therefore sees no error and continues on to the sibling subtree. Breaking at
|
||||
`tree[3]` in pre-order still visits all 7 nodes. Fix by splitting the recursion: an
|
||||
internal helper that propagates `AKERR_ITERATOR_BREAK` unhandled, and a public entry point
|
||||
that swallows it exactly once at the top. `tests/test_tree.c` currently passes only
|
||||
because the search target is the last node in all three orders.
|
||||
**What closing it would touch.** A `FILE *` over a custom stream — `fopencookie`
|
||||
on glibc, `funopen` on the BSDs — whose read function returns a short count
|
||||
without setting either flag. That is a platform-specific test helper in
|
||||
`tests/aksl_capture.h` guarded on the platform, which is why it has not been
|
||||
written yet rather than an oversight.
|
||||
|
||||
4. **`va_end` is never called.** `src/stdlib.c:96`, `:109`, `:122` each call `va_start` with
|
||||
no matching `va_end` on any path — happy or error. This is undefined behaviour per the C
|
||||
standard and leaks register-save state on some ABIs.
|
||||
### 2.2 The string-buffer overflow guard is untestable
|
||||
|
||||
5. **`aksl_atoi`/`atol`/`atoll`/`atof` cannot report a conversion failure.**
|
||||
`src/stdlib.c:135`–`169`. `atoi("not a number")` returns success with `0`;
|
||||
`atoi("99999999999999999999")` returns success with a wrapped value. Since the entire
|
||||
point of this library is turning silent libc failures into error contexts, these should be
|
||||
reimplemented over `strtol`/`strtoll`/`strtod` with `errno = 0` before the call, an
|
||||
`endptr` check for "no digits consumed" and "trailing junk", and a range check —
|
||||
raising `AKERR_VALUE` and `ERANGE` respectively. Keep the `atoi`-compatible names but
|
||||
document the stricter contract, or add `aksl_strtol`-family wrappers alongside.
|
||||
`tests/test_convert_strict.c` asserts that contract and is registered as a known failure.
|
||||
`src/collections.c`, the `capacity = needed` arm in `strbuf_reserve`. Reaching it
|
||||
needs an `aksl_StrBuf` within a factor of two of `SIZE_MAX`, which is not a test,
|
||||
it is a hang. The other two uncovered lines.
|
||||
|
||||
6. **`aksl_realpath` mishandles `resolved_path`.** `src/stdlib.c:171`.
|
||||
- `resolved_path` is never NULL-checked. `realpath(path, NULL)` is valid and mallocs a
|
||||
buffer, but the wrapper discards `result`, so the caller gets nothing and the buffer
|
||||
leaks.
|
||||
- On failure the message formats `resolved_path` with `%s` while `realpath` leaves it
|
||||
unspecified — for the normal caller who passed an uninitialised stack buffer, the error
|
||||
path itself reads uninitialised memory and can crash.
|
||||
- There is no way to express the buffer's size, so callers must know to supply `PATH_MAX`
|
||||
bytes. Consider `aksl_realpath(const char *path, char *buf, size_t buflen)`, or an
|
||||
allocating variant that returns the malloc'd pointer through an out-param.
|
||||
**Consequence.** None known. It is there because doubling a capacity is a
|
||||
multiplication, and an unguarded one is how a growable buffer turns into a heap
|
||||
overflow.
|
||||
|
||||
### 2.2 Latent issues and API contract gaps
|
||||
**What closing it would touch.** Nothing worth doing. Recorded so the coverage
|
||||
listing does not read as an oversight.
|
||||
|
||||
1. **`errno` is used as an error status where it may be stale.** `aksl_malloc` reports
|
||||
`errno` when `malloc` returns NULL, but `malloc(0)` may legitimately return NULL without
|
||||
setting `errno`, producing a `FAIL` with `status == 0` — an error context that every
|
||||
`DETECT`/`CATCH` will read as *success* while still holding a pool slot. Guard every
|
||||
`errno`-sourced status with a fallback (`errno ? errno : AKERR_IO`) and set `errno = 0`
|
||||
before the call being wrapped.
|
||||
### 2.3 `aksl_version_check()` ignores its `patch` argument
|
||||
|
||||
2. **`aksl_fopen` does not validate `pathname` or `mode`.** `fopen(NULL, ...)` is UB. Add
|
||||
`AKERR_NULLPOINTER` guards.
|
||||
`src/stdlib.c`, the `(void)patch`. Correct for the current "same soname" rule —
|
||||
patch level never breaks the ABI — but the parameter exists only so the error
|
||||
message can name the caller's full version.
|
||||
|
||||
3. **`aksl_fread`/`aksl_fwrite` lose the transfer count and hide short transfers.**
|
||||
- `ptr` is never NULL-checked in either function.
|
||||
- Neither has an out-param for the number of members actually transferred, so a caller who
|
||||
gets `AKERR_EOF` cannot tell how much data arrived. Add `size_t *nmemb_out`.
|
||||
- If `nmemr != nmemb` but neither `feof` nor `ferror` is set, both functions fall through
|
||||
to `SUCCEED_RETURN` — a short transfer reported as complete success.
|
||||
- `aksl_fwrite`'s error message reads `"Error reading file"` (`src/stdlib.c:83`), and
|
||||
checking `feof()` on a write path is meaningless.
|
||||
**Consequence.** None today. If a future compatibility rule needs the patch level
|
||||
to participate, that is the line to change, and the `#if` in
|
||||
`tests/test_version.c` is the test that encodes the rule.
|
||||
|
||||
4. **`aksl_sprintf` wraps `vsprintf`, which is unbounded.** There is no way for a caller to
|
||||
bound the destination. Add `aksl_snprintf`/`aksl_vsnprintf` and consider deprecating the
|
||||
`sprintf` wrapper, or reject it outright — an "error-handling" wrapper around an
|
||||
unbounded write is a sharp edge the library exists to remove.
|
||||
### 2.4 Four uncovered `HANDLE(e, AKERR_ITERATOR_BREAK)` lines
|
||||
|
||||
5. **No `format` attribute on the variadic wrappers.** Adding
|
||||
`__attribute__((format(printf, 2, 3)))` (and the equivalents) restores the compile-time
|
||||
format/argument checking that callers lose by going through the wrapper.
|
||||
|
||||
6. **`aksl_strhash_djb2` sign-extends bytes ≥ 0x80.** `src/stdlib.c:181` iterates a
|
||||
`char *`, which is signed on x86/ARM Linux, so high bytes contribute a sign-extended
|
||||
negative value and the hash differs from the canonical djb2 — and differs across
|
||||
platforms where `char` is unsigned. Iterate a `const unsigned char *`. Also take
|
||||
`const char *` in the signature so callers need not cast away constness.
|
||||
|
||||
7. **`aksl_tree_iterate` recursion is unbounded and cycle-blind.** A deep or degenerate tree
|
||||
overflows the stack, and a child pointer that loops back to an ancestor recurses forever.
|
||||
Add a depth limit (raising `AKERR_OUTOFBOUNDS`) and/or a visited check raising
|
||||
`AKERR_CIRCULAR_REFERENCE`, consistent with what the list functions already do.
|
||||
|
||||
8. **`lalloc`, `lfree` and `queue` are dead parameters.** `lalloc`/`lfree` are defaulted and
|
||||
then never called; `queue` is entirely unused (it is the only `-Wunused-parameter` warning
|
||||
in the file). The doc comment even tells the caller to "pass NULL here", which is a sign
|
||||
the queue belongs in an internal helper rather than the public signature. Either
|
||||
implement BFS so they are used, or drop them from the public API until it is.
|
||||
|
||||
9. **Unknown `searchmode` values return success.** The `switch` in `aksl_tree_iterate` has no
|
||||
`default:`; `searchmode == 99` and the header-defined `AKSL_TREE_SEARCH_VISIT` (5) both
|
||||
fall straight through to `SUCCEED_RETURN` having visited nothing. Add
|
||||
`default: FAIL_RETURN(e, AKERR_VALUE, ...)`.
|
||||
|
||||
10. **`AKSL_TREE_SEARCH_BFS`/`BFS_RIGHT` are unimplemented** (`AKERR_NOT_IMPLEMENTED`), and
|
||||
`AKSL_TREE_SEARCH_VISIT` is declared in the header with a documented meaning but no
|
||||
implementation anywhere.
|
||||
|
||||
11. **`aksl_memset`'s and `aksl_memcpy`'s inner checks are dead code.** `memset` returns `s`
|
||||
and `memcpy` returns `d`, both unconditionally; neither can fail. The
|
||||
`FAIL_ZERO_RETURN(e, memset(...), errno, ...)` and `(memcpy(...) == d)` checks can never
|
||||
fire and only obscure the intent. Also, `memcpy` with overlapping ranges is UB — either
|
||||
document that or dispatch to `memmove`.
|
||||
|
||||
12. **`aksl_list_pop` cannot tell the caller the new head.** Popping the head leaves the
|
||||
caller's head pointer dangling at a now-detached node. Add an out-param for the new head,
|
||||
or a `aksl_list_pop(aksl_ListNode **head, aksl_ListNode *node)` form.
|
||||
|
||||
13. **`aksl_free` does not clear the caller's pointer**, so double-free remains easy. Consider
|
||||
an `aksl_freep(void **ptr)` that frees and NULLs.
|
||||
|
||||
14. **No initialisers for the public structs.** Every caller must remember to `memset` an
|
||||
`aksl_ListNode`/`aksl_TreeNode` to zero before use (both existing tests do). Add
|
||||
`aksl_list_node_init`/`aksl_tree_node_init` or `AKSL_LIST_NODE_INIT` macros.
|
||||
|
||||
15. **`aksl_TreeNode.parent` is declared but never set or read** by any library function.
|
||||
|
||||
16. **Header hygiene**: no `extern "C" { }` guard for C++ consumers; `akstdlib.h` pulls in
|
||||
`stdio.h`/`stdlib.h`/`string.h`/`stdint.h` into every consumer's namespace. The version
|
||||
macros this item also asked for are **done**: `AKSL_VERSION_MAJOR`/`_MINOR`/`_PATCH`/
|
||||
`_STRING`/`_NUMBER`/`_SONAME` are generated into `akstdlib_version.h` from
|
||||
`project(akstdlib VERSION …)`, with `aksl_version()`/`aksl_version_string()`/
|
||||
`aksl_version_soname()` and `AKSL_VERSION_CHECK()` reporting the *loaded* library so a
|
||||
header/`.so` mismatch is nameable. See `tests/test_version.c` and `README.md`.
|
||||
|
||||
17. **Doxygen coverage is 2 functions out of 24.** A `Doxyfile` exists but only
|
||||
`aksl_tree_iterate` and `aksl_list_iterate` have doc comments. Every public function
|
||||
needs `@param`/`@throws`/`@return`, especially the ones whose contract deviates from libc
|
||||
(`aksl_free(NULL)` is an error; `aksl_atoi` will become strict). The four
|
||||
`aksl_version_*` entry points have prose comments in the header but no Doxygen tags.
|
||||
|
||||
### 2.3 Build, CI and repository
|
||||
|
||||
- [ ] **`CMakeLists.txt:4-6` sets `CMAKE_CXX_FLAGS` in a C-only project**, so `-g -ggdb -pg`
|
||||
never reach the C compiler. Use `CMAKE_C_FLAGS` — or better, drop `-pg` from the
|
||||
default build entirely (it is what produces the stray `gmon.out` sitting untracked in
|
||||
the repo root) and let `CMAKE_BUILD_TYPE` control debug info.
|
||||
- [ ] **No warning flags.** Add `-Wall -Wextra` (and ideally `-Werror` in CI). The only
|
||||
current warning is the unused `queue` parameter, so the cost of turning them on is low.
|
||||
- [ ] `src/stdlib.c` triggers **"ISO C99 requires at least one argument for the `...`"** on
|
||||
roughly 20 lines under `-Wpedantic`, because `FAIL_*` is called with a bare message and
|
||||
no varargs. Either always pass an argument, or add a zero-arg-safe form in libakerror.
|
||||
- [x] **The `deps/libakerror` submodule was pinned 22 commits behind `main`** (pinned at
|
||||
`4fad0ce "Add gitea workflow"`). Bumped to `5ff8790`, which is libakerror **1.0.0** —
|
||||
the release that made the status-name table private, moved consumer status codes to a
|
||||
band starting at `AKERR_FIRST_CONSUMER_STATUS` (256), made range ownership enforced,
|
||||
and gave the library an soname. See `deps/libakerror/UPGRADING.md`. The bump also
|
||||
brings the refcount-leak fix, the stack-trace buffer-overflow fix and the
|
||||
format-string fix. Nothing in this library's sources used the removed
|
||||
`AKERR_MAX_ERR_VALUE`, `__AKERR_ERROR_NAMES`, `AKERR_STATUS_RANGE_OK` or
|
||||
`AKERR_STATUS_NAME_OK`, so the source change was confined to the build, the packaging
|
||||
and one compile-time guard. What it did move is recorded in the three items below.
|
||||
- [ ] **libakerror does not namespace its `coverage` target when embedded**, only its
|
||||
`mutation` target (`deps/libakerror/CMakeLists.txt:172` vs `:189`). A
|
||||
`-DAKSL_COVERAGE=ON` top-level build therefore failed to configure at all —
|
||||
*"another target with the same name already exists"* — the moment the dependency
|
||||
gained a coverage target. Worked around in `CMakeLists.txt` by shadowing
|
||||
`add_custom_target` for the duration of the `add_subdirectory()` call and renaming the
|
||||
dependency's to `akerror_coverage`, alongside the existing `add_test` shadow. **Fix
|
||||
upstream and delete the workaround**: libakerror should apply the same
|
||||
`CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR` test to `coverage` that it
|
||||
already applies to `mutation`.
|
||||
- [ ] **Branch coverage of `src/stdlib.c` fell from 51.0% to 44.3% on the 1.0.0 bump**, with
|
||||
no change to this library's sources or tests. Line coverage held at 99.0% (200/202)
|
||||
and function coverage at 100% (21/21); the branch *denominator* went from 661 to 1087
|
||||
because the 1.0.0 `PREPARE_ERROR`/`FAIL_*` macros expand to more branches at every
|
||||
call site. 337/661 became 481/1087 — 144 more branches covered, 426 more counted. The
|
||||
CI branch gate was re-ratcheted 45 → 40 to match. Most of the added branches are
|
||||
unreachable from the way this library calls the macros, so raising the number here
|
||||
would mean testing libakerror's macros rather than this library; that belongs to
|
||||
libakerror's mutation suite, since macros expand at the call site and coverage cannot
|
||||
see them properly from either side. Revisit if the gap ever hides a real regression.
|
||||
- [x] **`project()` declared no `VERSION`**, so `@PROJECT_VERSION@` expanded to nothing, the
|
||||
installed `akstdlib.pc` shipped an empty `Version:` field, and `libakstdlib.so` carried
|
||||
no soname. Now `project(akstdlib VERSION 0.1.0)`, with `SOVERSION` `0.1`
|
||||
(`MAJOR.MINOR` while major is 0, `MAJOR` from 1.0 — the `if()` in `CMakeLists.txt` and
|
||||
the matching `#if` in `tests/test_version.c` encode the rule and are tested against
|
||||
each other), an `akstdlibConfigVersion.cmake` at `SameMinorVersion`, and version macros
|
||||
generated into `akstdlib_version.h`. Verified: `find_package(akstdlib 0.1)` is accepted
|
||||
while `0.2` and `1.0` are refused, and a 0.2.0 build dropped in under the 0.1 filename
|
||||
is caught by `AKSL_VERSION_CHECK()`.
|
||||
- [ ] **libakerror still installs no `akerrorConfigVersion.cmake`**, so
|
||||
`cmake/akstdlib.cmake.in` has to call `find_dependency(akerror)` with no version — a
|
||||
request for one would be refused for want of a version file regardless of what is
|
||||
installed. libakstdlib now does this correctly and libakerror does not; fix it there
|
||||
with the same `write_basic_package_version_file()` call, then add the `1.0.0` floor to
|
||||
the `find_dependency` here. Until then the floor rests on `akstdlib.pc`'s `Requires:`
|
||||
and the `#error` guard in `akstdlib.h`.
|
||||
- [ ] **`aksl_version_check()` ignores its `patch` argument** (`src/stdlib.c`, the `(void)patch`),
|
||||
which is correct for the current "same soname" rule but means the parameter exists only
|
||||
so the error message can name the caller's full version. If a future rule needs patch
|
||||
to participate, that is the line to change.
|
||||
- [ ] **CI does not build against the submodule it pins.** `.gitea/workflows/ci.yaml` clones
|
||||
`libakerror@main` and installs it, while the build it then runs is top-level and so
|
||||
compiles `deps/libakerror` at the pinned commit — two different libakerror versions
|
||||
depending on where you build, and the installed one is never actually linked. Pick
|
||||
one. (The submodule is at least *present* in CI now: §1.0 added
|
||||
`submodules: recursive` to the checkout, without which configure failed outright.)
|
||||
- [x] **`ctest` was red** (`tree` failing plus four *Not Run* submodule tests) — fixed in
|
||||
§1.0. The build badge in `README.md` means something again.
|
||||
- [ ] **Untracked build litter** in the repo root: `build/`, `gmon.out`,
|
||||
`CMakeLists.txt-akstdlib`, and a dozen `*~` editor backups. Add a `.gitignore`
|
||||
(libakerror has one; this repo does not).
|
||||
- [ ] **`README.md` is a build badge and nothing else.** It needs at minimum: what the
|
||||
library is, the akerror wrapper contract, the list of wrapped functions, and the
|
||||
deviations from libc semantics.
|
||||
- [ ] **Indentation is inconsistent** — `src/stdlib.c` mixes hard tabs and 4-space indents
|
||||
within the same functions. libakerror standardised on Stroustrup style
|
||||
(commit `e5f7616`); apply the same here, ideally with a checked-in `.clang-format`.
|
||||
Macro artifacts rather than gaps. In libakerror that macro begins with the
|
||||
`break;` belonging to `PROCESS`'s `case 0:` arm, reachable only when a callback
|
||||
returns a non-NULL context whose status is *zero* — which is the pathological case
|
||||
the errno-fallback work removed. Left uncovered deliberately rather than pinned
|
||||
by a test that would have to manufacture it.
|
||||
|
||||
---
|
||||
|
||||
## 3. libc functions not yet wrapped
|
||||
## 3. Deliberate omissions
|
||||
|
||||
Ordered roughly by how much a caller of this library would miss them. Each entry means "add
|
||||
an `aksl_`-prefixed wrapper that turns the documented failure modes into an
|
||||
`akerr_ErrorContext`".
|
||||
Recorded so nobody adds them thinking they were forgotten. Each is a decision,
|
||||
and each can be revisited with an argument.
|
||||
|
||||
### 3.1 High priority — gaps in areas the library already covers
|
||||
| Not wrapped | Why |
|
||||
|---|---|
|
||||
| `sprintf`, `vsprintf` | Cannot be bounded. An error-handling wrapper around an unbounded write is the sharp edge this library exists to remove. `aksl_snprintf` and `aksl_asprintf` cover both real uses. |
|
||||
| `strtok` | Keeps its state in a hidden static, so two interleaved tokenisations corrupt each other silently and any use from a thread is a bug. `aksl_strtok_r` and `aksl_strsep` cover it. |
|
||||
| `strcpy`, `strcat` as libc spells them | Cannot be called safely without the destination's size. The wrappers take it. |
|
||||
| `setbuf` | Exactly `setvbuf(stream, buf, buf ? _IOFBF : _IONBF, BUFSIZ)` and strictly less expressive. Wrapping it would add a second way to say one thing. |
|
||||
| `perror` | Writes to stderr and consults a global. `aksl_strerror` is the akerror-native equivalent and knows this library's own statuses as well as errno's. |
|
||||
| `strerror_r` | Two incompatible functions share that name and which one you get depends on feature-test macros a consumer cannot influence from inside this header. `aksl_strerror` is built on libakerror's registry instead. |
|
||||
|
||||
**Memory (`stdlib.h`, `string.h`)**
|
||||
- [ ] `calloc`, `realloc`, `reallocarray` — `realloc`'s "returns NULL and the old pointer is
|
||||
still valid" trap is exactly what this library should be hiding.
|
||||
- [ ] `aligned_alloc`, `posix_memalign`
|
||||
- [ ] `memmove`, `memcmp`, `memchr`
|
||||
---
|
||||
|
||||
**Bounded string formatting (`stdio.h`)**
|
||||
- [ ] `snprintf`, `vsnprintf` — needed to make §2.2.4 fixable.
|
||||
- [ ] `vprintf`, `vfprintf`, `vsprintf` — the `va_list` forms, so consumers can build their
|
||||
own variadic wrappers on top.
|
||||
- [ ] `asprintf` / `vasprintf` (GNU) as an allocating alternative.
|
||||
## 4. Not yet wrapped
|
||||
|
||||
**String → number (`stdlib.h`)**
|
||||
- [ ] `strtol`, `strtoll`, `strtoul`, `strtoull`, `strtod`, `strtof`, `strtold` — the correct
|
||||
foundation for §2.1.5.
|
||||
Ordered by how much a caller of this library would miss them. §3.1 and §3.6 of
|
||||
the old numbering are done; what follows is what is left.
|
||||
|
||||
**Strings (`string.h`)**
|
||||
- [ ] `strlen`, `strnlen`
|
||||
- [ ] `strcpy`, `strncpy`, `strcat`, `strncat` — with truncation reported as an error rather
|
||||
than silently accepted, which is the whole value proposition here.
|
||||
- [ ] `strdup`, `strndup`
|
||||
- [ ] `strcmp`, `strncmp`, `strcasecmp`, `strncasecmp`, `strcoll`
|
||||
- [ ] `strchr`, `strrchr`, `strstr`, `strcasestr`, `strpbrk`, `strspn`, `strcspn`
|
||||
- [ ] `strtok_r`, `strsep`
|
||||
- [ ] `strerror_r`
|
||||
### 4.1 POSIX file and process API
|
||||
|
||||
**Stream I/O (`stdio.h`)**
|
||||
- [ ] `fseek`, `ftell`, `rewind`, `fgetpos`, `fsetpos`, `fseeko`, `ftello`
|
||||
- [ ] `fflush`
|
||||
- [ ] `fgets`, `fputs`, `fgetc`/`getc`/`getchar`, `fputc`/`putc`/`putchar`, `ungetc`
|
||||
- [ ] `getline`, `getdelim`
|
||||
- [ ] `freopen`, `fdopen`, `fileno`
|
||||
- [ ] `setvbuf`, `setbuf`
|
||||
- [ ] `clearerr`, `feof`, `ferror` — thin, but worth exposing so callers never touch raw
|
||||
`FILE *` internals.
|
||||
- [ ] `sscanf`, `fscanf`, `scanf` — the return-value semantics (items matched vs `EOF`) are a
|
||||
classic silent-failure source.
|
||||
- [ ] `remove`, `rename`, `tmpfile`, `mkstemp`, `mkdtemp`
|
||||
- [ ] `perror` — or an akerror-native equivalent.
|
||||
|
||||
### 3.2 POSIX file and process API (likely the next major surface)
|
||||
The most likely next surface. Nothing here is blocked; it is simply not written.
|
||||
|
||||
**`unistd.h` / `fcntl.h`**
|
||||
- [ ] `open`, `close`, `read`, `write`, `pread`, `pwrite`, `lseek`
|
||||
@@ -563,153 +186,144 @@ an `aksl_`-prefixed wrapper that turns the documented failure modes into an
|
||||
- [ ] `sysconf`, `pathconf`
|
||||
- [ ] `sleep`, `usleep`, `nanosleep`
|
||||
|
||||
The short-read/short-write contract is the interesting part: `read(2)` returning
|
||||
fewer bytes than asked for is *normal* on a pipe or a socket and a failure on a
|
||||
regular file, so the wrapper needs the same transferred-count out-param
|
||||
`aksl_fread` has, and callers need to be told which case they are in.
|
||||
|
||||
**`sys/stat.h`**
|
||||
- [ ] `stat`, `fstat`, `lstat`, `fstatat`
|
||||
- [ ] `statvfs`, `fstatvfs`
|
||||
|
||||
**`dirent.h`**
|
||||
- [ ] `opendir`, `fdopendir`, `readdir`, `readdir_r`, `closedir`, `rewinddir`, `scandir`
|
||||
- [ ] `opendir`, `fdopendir`, `readdir`, `closedir`, `rewinddir`, `scandir`
|
||||
|
||||
`readdir(3)` returning NULL for both "end of directory" and "error, check errno"
|
||||
is the same conflation `aksl_fgetc` and `aksl_fgets` already untangle, and should
|
||||
be untangled the same way: AKERR_EOF for the end, the errno for the error.
|
||||
|
||||
**Process control**
|
||||
- [ ] `fork`, `execve` / `execvp` / `execl` family, `waitpid`, `wait`
|
||||
- [ ] `fork`, the `exec*` family, `waitpid`, `wait`
|
||||
- [ ] `posix_spawn`
|
||||
- [ ] `system`, `popen`, `pclose`
|
||||
- [ ] `getpid`, `getppid`, `getuid`, `geteuid`, `setuid`, `setgid`
|
||||
- [ ] `atexit`, `exit`, `_exit`, `abort` — mostly to give akerror a hook on shutdown.
|
||||
- [ ] `atexit`, `exit`, `_exit`, `abort` — mostly to give akerror a shutdown hook
|
||||
- [ ] `getenv`, `setenv`, `unsetenv`, `putenv`, `clearenv`
|
||||
|
||||
`fork` needs thinking about before it is wrapped: the error pool is inherited by
|
||||
the child, and any context live at the moment of the fork exists twice
|
||||
afterwards.
|
||||
|
||||
**`sys/mman.h`**
|
||||
- [ ] `mmap`, `munmap`, `mprotect`, `msync`, `madvise`
|
||||
|
||||
### 3.3 Time
|
||||
### 4.2 Time
|
||||
|
||||
- [ ] `time`, `clock_gettime`, `clock_getres`, `gettimeofday`
|
||||
- [ ] `localtime_r`, `gmtime_r`, `mktime`, `timegm`, `difftime`
|
||||
- [ ] `strftime`, `strptime`
|
||||
- [ ] `clock`, `times`
|
||||
|
||||
### 3.4 Sorting, searching and misc `stdlib.h`
|
||||
`strftime(3)` returns 0 for both "the output was empty" and "it did not fit",
|
||||
which is the bounded-write ambiguity `aksl_snprintf` already resolves.
|
||||
|
||||
- [ ] `qsort`, `qsort_r`, `bsearch` — comparator errors currently have nowhere to go; an
|
||||
akerror-aware comparator signature would be a genuine improvement.
|
||||
- [ ] `abs`, `labs`, `llabs`, `div`, `ldiv`, `lldiv`
|
||||
### 4.3 Sorting, searching and the rest of `stdlib.h`
|
||||
|
||||
- [ ] `qsort`, `qsort_r`, `bsearch` — a comparator that fails currently has
|
||||
nowhere to put the error. An akerror-aware comparator signature, shaped
|
||||
like the `aksl_TreeCompareFunc` the tree functions already take, would be a
|
||||
genuine improvement over libc rather than a wrapper around it.
|
||||
- [ ] `abs`, `labs`, `llabs`, `div`, `ldiv`, `lldiv` — note that
|
||||
`abs(INT_MIN)` is undefined behaviour, which is exactly the kind of silent
|
||||
trap worth surfacing.
|
||||
- [ ] `rand`, `srand`, `random`, `srandom`, `getrandom`/`arc4random`
|
||||
|
||||
### 3.5 Lower priority / larger projects
|
||||
### 4.4 Larger surfaces
|
||||
|
||||
- [ ] **Sockets**: `socket`, `bind`, `listen`, `accept`, `connect`, `send`/`sendto`/`sendmsg`,
|
||||
`recv`/`recvfrom`/`recvmsg`, `shutdown`, `setsockopt`/`getsockopt`, `getaddrinfo`/
|
||||
`freeaddrinfo`/`gai_strerror`, `inet_ntop`/`inet_pton`
|
||||
- [ ] **Multiplexing**: `select`, `poll`, `ppoll`, `epoll_create1`/`epoll_ctl`/`epoll_wait`
|
||||
- [ ] **Signals**: `sigaction`, `sigprocmask`, `sigemptyset`/`sigaddset`, `kill`, `raise`,
|
||||
`signalfd`
|
||||
- [ ] **Threads**: `pthread_create`/`join`/`detach`, `pthread_mutex_*`, `pthread_cond_*`,
|
||||
`pthread_rwlock_*`, `sem_*` — blocked on resolving the thread-safety question in §1.9
|
||||
(libakerror's error pool is an unlocked process-global array).
|
||||
- [ ] **Dynamic loading**: `dlopen`, `dlsym`, `dlclose`, `dlerror`
|
||||
- [ ] **Sockets**: `socket`, `bind`, `listen`, `accept`, `connect`, the
|
||||
`send`/`recv` families, `shutdown`, `setsockopt`/`getsockopt`,
|
||||
`getaddrinfo`/`freeaddrinfo`/`gai_strerror`, `inet_ntop`/`inet_pton`.
|
||||
`getaddrinfo` is the interesting one: it has its own error space, neither
|
||||
errno nor akerror, which needs mapping into a registered status range.
|
||||
- [ ] **Multiplexing**: `select`, `poll`, `ppoll`, `epoll_*`
|
||||
- [ ] **Signals**: `sigaction`, `sigprocmask`, `sigemptyset`/`sigaddset`, `kill`,
|
||||
`raise`, `signalfd`. A handler cannot raise an akerror context — the pool is
|
||||
not async-signal-safe — so the wrapper covers installation and masking only,
|
||||
and that limit should be documented rather than discovered.
|
||||
- [ ] **Threads**: `pthread_create`/`join`/`detach`, `pthread_mutex_*`,
|
||||
`pthread_cond_*`, `pthread_rwlock_*`, `sem_*`. **Blocked on §1.1.**
|
||||
- [ ] **Dynamic loading**: `dlopen`, `dlsym`, `dlclose`, `dlerror`. `dlerror(3)`
|
||||
is the same "returns a string and clears itself" trap as `strerror`.
|
||||
- [ ] **Locale / wide chars**: `setlocale`, `mbstowcs`, `wcstombs`, `iconv_*`
|
||||
- [ ] **Math**: the `math.h` functions that set `errno`/raise FP exceptions (`sqrt`, `log`,
|
||||
`pow`, `acos`, …) — probably better served by a dedicated `libakmath`.
|
||||
- [ ] **Math**: the `math.h` functions that set `errno` or raise FP exceptions
|
||||
(`sqrt`, `log`, `pow`, `acos`, …). Probably a dedicated `libakmath` rather
|
||||
than more surface here — the failure model is `fetestexcept`, not errno, and
|
||||
it does not resemble anything else in this library.
|
||||
|
||||
### 3.6 Non-libc additions the current data structures imply
|
||||
### 4.5 Data structures
|
||||
|
||||
Not libc wrappers, but the existing list/tree API is visibly incomplete:
|
||||
The list and tree API is complete against what §3.6 asked for. What a second
|
||||
consumer might want next:
|
||||
|
||||
- [ ] List: `prepend`, `insert_after`/`insert_before`, `length`, `find`, `reverse`,
|
||||
`concat`, `free_all`, reverse iteration, and a head/tail-tracking container type so
|
||||
`append` is O(1) instead of O(n).
|
||||
- [ ] Tree: `insert`, `remove`, `find`, `height`, `count`, `free_all`, and the BFS traversal
|
||||
the `lalloc`/`lfree`/`queue` parameters were designed for.
|
||||
- [ ] Hash map built on `aksl_strhash_djb2`, plus additional hashes (FNV-1a, xxHash) and a
|
||||
NUL-terminated `aksl_strhash_djb2_str` convenience form.
|
||||
- [ ] Growable buffer / string-builder type, to make the `snprintf` and `strcat` wrappers
|
||||
pleasant to use.
|
||||
- [ ] A dynamic array / vector, on the same "caller owns the storage" terms as
|
||||
`aksl_HashMap`.
|
||||
- [ ] A hash map keyed on something other than a string — the current one copies
|
||||
keys into fixed slots, which is right for identifiers and wrong for
|
||||
anything large or binary.
|
||||
- [ ] Balanced insertion for the tree. It is a plain unbalanced BST and says so;
|
||||
sorted input gives a degenerate chain that `AKSL_TREE_MAX_DEPTH` then
|
||||
refuses. Bounded rather than dangerous, but a red-black or AVL variant is
|
||||
the honest fix if anyone inserts sorted data in earnest.
|
||||
|
||||
## 4. Evidence from the first full consumer
|
||||
---
|
||||
|
||||
`akbasic` (`source.starfort.tech/andrew/akbasic`) is a ~6,300-line C interpreter built on this
|
||||
library and `libakerror`. It is the first consumer to exercise the whole surface rather than a
|
||||
corner of it, so what it *could not* use is worth recording: it prioritizes the wishlist in §3
|
||||
by what a real port actually reaches for, and it confirms which §2 entries have teeth.
|
||||
## 5. Evidence from the first full consumer
|
||||
|
||||
**The headline number.** Across `src/`, akbasic makes **10 calls into this library** and
|
||||
**116 calls to raw libc**:
|
||||
`akbasic` (`source.starfort.tech/andrew/akbasic`) is a ~6,300-line C interpreter
|
||||
built on this library and `libakerror`. It was the first consumer to exercise the
|
||||
whole surface rather than a corner of it, and what it *could not* use is what
|
||||
prioritised the work above.
|
||||
|
||||
| `aksl_*` used | Count |
|
||||
|---|---|
|
||||
| `aksl_fopen` / `aksl_fclose` | 6 |
|
||||
| `aksl_fprintf` | 2 |
|
||||
| `aksl_fwrite` | 1 |
|
||||
| `aksl_strhash_djb2` | 1 |
|
||||
**The number that started it.** Across `src/`, akbasic made **10 calls into this
|
||||
library and 116 to raw libc** — a library whose value proposition is "turn silent
|
||||
libc failures into error contexts", bypassed 92% of the time by the consumer most
|
||||
committed to it.
|
||||
|
||||
| Raw libc it had to use instead | Count | Wanted from §3 |
|
||||
| Raw libc it had to use | Count | Now available as |
|
||||
|---|---|---|
|
||||
| `snprintf` | 28 | §3.1 bounded string formatting |
|
||||
| `strlen` | 37 | §3.1 strings |
|
||||
| `strcmp` | 16 | §3.1 strings |
|
||||
| `strncpy` | 15 | §3.1 strings |
|
||||
| `memcpy` / `memset` | 16 | wrapped, but see §2.2.11 |
|
||||
| `strtoll` / `strtod` | 2 | §3.1 string → number |
|
||||
| `fgets` | 2 | §3.1 stream I/O |
|
||||
| `strstr` | 1 | §3.1 strings |
|
||||
| `strlen` | 37 | `aksl_strlen` |
|
||||
| `snprintf` | 28 | `aksl_snprintf` |
|
||||
| `strcmp` | 16 | `aksl_strcmp` |
|
||||
| `memcpy` / `memset` | 16 | `aksl_memcpy` / `aksl_memset` |
|
||||
| `strncpy` | 15 | `aksl_strncpy` |
|
||||
| `strtoll` / `strtod` | 2 | `aksl_strtoll` / `aksl_strtod` |
|
||||
| `fgets` | 2 | `aksl_fgets` |
|
||||
| `strstr` | 1 | `aksl_strstr` |
|
||||
|
||||
A library whose value proposition is "turn silent libc failures into error contexts" is being
|
||||
bypassed 92% of the time by the consumer most committed to it. Every one of those calls is a
|
||||
place where a failure goes unreported.
|
||||
**All four things the port had to write for itself now exist here.**
|
||||
|
||||
### 4.1 What the port had to write for itself
|
||||
1. **A strict `strtoll`/`strtod` wrapper** (`akbasic/src/convert.c`, ~60 lines).
|
||||
The `aksl_strto*` family is that, with the endptr/`errno`/range contract.
|
||||
akbasic's own `TODO.md` §1.9 formally bans the `aksl_ato*` family because
|
||||
routing four diagnosable errors through it would have turned them into wrong
|
||||
answers — `VAL("garbage")` silently returning `0.0`. That ban can be lifted:
|
||||
the `ato*` forms report failures now. **`src/convert.c` can be deleted.**
|
||||
2. **A fixed-capacity string-keyed hash table** (`akbasic/src/symtab.c`, ~130
|
||||
lines, needed three times over). `aksl_hashmap_*` is that table generalised,
|
||||
with tombstones on delete, which the original did not have.
|
||||
3. **The bounded-copy-with-truncation-as-error idiom, at ten sites.**
|
||||
`aksl_strcpy` and `aksl_strncpy` are exactly that idiom.
|
||||
4. **Uppercase folding for case-insensitive lookup, three times.**
|
||||
`aksl_strcasecmp` and `aksl_strncasecmp`.
|
||||
|
||||
1. **A strict `strtoll`/`strtod` wrapper** (`akbasic/src/convert.c`, ~60 lines). This is §2.1.5
|
||||
biting, and it is not cosmetic: the Go reference this port reproduces checks `strconv`'s
|
||||
error at four sites and turns each into a BASIC error. Routing them through `aksl_atoi`
|
||||
would have converted four *diagnosable errors* into *wrong answers* — `VAL("garbage")`
|
||||
silently returning `0.0` rather than raising, and a malformed line number becoming line 0.
|
||||
akbasic's own `TODO.md` §1.9 formally bans the `aksl_ato*` family for this reason. **The
|
||||
fix wanted is exactly §3.1's `strtol` family plus the endptr/`errno`/range contract §2.1.5
|
||||
describes**; when it lands, `src/convert.c` gets deleted.
|
||||
**And the four confirmed-with-impact items are closed.** §2.2.4's unbounded
|
||||
`aksl_sprintf` is gone; §2.2.2's unchecked `aksl_fopen` arguments are checked, so
|
||||
`akbasic_cmd_dload`'s hand-rolled validation and its comment pointing here can go;
|
||||
§2.2.6's sign-extended djb2 reads bytes unsigned; and §2.1.4's missing `va_end` —
|
||||
which akbasic's stdio text sink ran on every line of program output — is fixed.
|
||||
|
||||
2. **A fixed-capacity string-keyed hash table** (`akbasic/src/symtab.c`, ~130 lines). §3.6
|
||||
already calls for a "hash map built on `aksl_strhash_djb2`". This is that, three times over:
|
||||
the interpreter needs one for variables, one for functions and one for labels per scope. It
|
||||
is open-addressed with linear probing over a caller-supplied slot count, refuses rather than
|
||||
resizes when full, and is the single most obviously-missing data structure in the library.
|
||||
Worth lifting more or less verbatim if you want a starting point.
|
||||
|
||||
3. **The bounded-copy-with-truncation-as-error idiom, ten times.** `strncpy` followed by an
|
||||
explicit NUL and a preceding length check appears at ten sites across six files. §3.1 lists
|
||||
`strncpy` "with truncation reported as an error rather than silently accepted, which is the
|
||||
whole value proposition here" — agreed, and the repetition is the evidence.
|
||||
|
||||
4. **Uppercase folding for case-insensitive lookup, three times.** Verb and function names are
|
||||
case-insensitive in BASIC while variable names are not, so the port folds case before every
|
||||
keyword lookup: by hand at `src/environment.c:197` and `src/parser_commands.c:113`, and
|
||||
through `<ctype.h>`'s `toupper` in `src/verbs.c`. §3.1's `strcasecmp`/`strncasecmp` would
|
||||
remove all three.
|
||||
|
||||
### 4.2 Confirmed with impact
|
||||
|
||||
- **§2.2.4 (`aksl_sprintf` is unbounded)** — akbasic never calls it, deliberately. Its 28
|
||||
`snprintf` calls all write into fixed buffers whose size is the whole point, and there is no
|
||||
`aksl_snprintf` to route them through. This is the single highest-value item in §3.1 for a
|
||||
consumer of this shape.
|
||||
- **§2.2.2 (`aksl_fopen` does not validate `pathname`/`mode`)** — real, and reached from user
|
||||
input. akbasic's `DLOAD`/`DSAVE` take a filename from a BASIC string expression, so an empty
|
||||
or unevaluated one is reachable from a running program; `akbasic_cmd_dload` validates the
|
||||
name itself before handing it over, with a comment pointing here.
|
||||
- **§2.2.6 (`aksl_strhash_djb2` sign-extends high bytes)** — benign for akbasic, which only
|
||||
ever hashes 7-bit ASCII identifiers, and noted in its §1.3 as a hazard for anyone who later
|
||||
keys a table on a filename or a string literal. Still worth fixing; the wrong answer is
|
||||
platform-dependent, which is the bad kind of benign.
|
||||
- **§2.1.4 (`va_end` is never called)** — akbasic's stdio text sink is built on `aksl_fprintf`
|
||||
and therefore runs this UB on every line of program output. Nothing has misbehaved on
|
||||
x86-64 glibc, which is exactly what makes it worth fixing before something does.
|
||||
|
||||
### 4.3 Not needed, and why
|
||||
|
||||
Recorded so the wishlist does not get prioritized purely by this consumer's shape. akbasic
|
||||
uses **no** allocator (§3.1 memory), **no** lists or trees (§3.6), and **no** `aksl_realpath`:
|
||||
it draws everything from fixed pools by design, and the `ak*` no-dynamic-allocation rule means
|
||||
`calloc`/`realloc` would go unused here even if they existed. A consumer that does allocate
|
||||
would weight §3.1's memory section far higher. The list and tree defects in §2.1.1–3 were
|
||||
avoided rather than survived — akbasic's symbol tables are open-addressed precisely because
|
||||
`aksl_list_append` truncates.
|
||||
**Still true, and still shaping the wishlist.** akbasic uses no allocator, no
|
||||
lists and no trees, drawing everything from fixed pools by design. A consumer that
|
||||
does allocate would weight §4.1's `open`/`read`/`write` far higher than this one
|
||||
does. The next thing worth doing is porting akbasic onto this release and
|
||||
counting the calls again.
|
||||
|
||||
Reference in New Issue
Block a user