Bump deps/libakerror 22 commits to 5ff8790 (1.0.0), which makes the status-name table private, moves consumer status codes to a band starting at AKERR_FIRST_CONSUMER_STATUS, enforces range ownership rather than treating it as advisory, and gives the library an soname. See deps/libakerror/UPGRADING.md. src/stdlib.c needed no changes. This library defines no status codes of its own -- it raises libakerror's AKERR_* codes and propagates errno, both inside libakerror's reserved 0-255 band -- and it never referenced AKERR_MAX_ERR_VALUE, __AKERR_ERROR_NAMES, AKERR_STATUS_RANGE_OK or AKERR_STATUS_NAME_OK. What moved was everything around the code: A -DAKSL_COVERAGE=ON build stopped configuring at all. libakerror namespaces its `mutation` target when embedded but not its `coverage` target, so it collided with ours. Shadow add_custom_target for the duration of the add_subdirectory() call and rename the dependency's to akerror_coverage, alongside the existing add_test shadow. Fix upstream and delete the workaround; recorded in TODO.md. Pin the 1.0.0 floor three ways, since no single one covers every consumption path: an #error in akstdlib.h feature-testing AKERR_FIRST_CONSUMER_STATUS, because libakerror publishes no version macro; Requires: akerror >= 1.0.0 in akstdlib.pc, which also gets consumers -lakerror transitively; and find_dependency(akerror) in akstdlibConfig.cmake. The last was already broken before this bump -- the template still carried its MyLibraryConfig placeholder with the dependency commented out, so any external find_package(akstdlib) failed with a bare "akerror::akerror not found" out of the generated targets file. Branch coverage of src/stdlib.c fell from 51.0% to 44.3% with no source or test change: the 1.0.0 PREPARE_ERROR/FAIL_* macros expand to more branches at every call site, so 337/661 became 481/1087 -- 144 more branches covered, 426 more counted. Line coverage held at 99.0% (200/202) and function coverage at 100% (21/21). Re-ratchet the CI branch gate 45 -> 40 rather than chase branches that belong to libakerror's own suite. tests/test_status_registry.c pins the contract that made the status-code migration a no-op: libakstdlib reserves no consumer range, so an application may allocate from AKERR_FIRST_CONSUMER_STATUS without coordinating with it, and every status this library raises is inside the reserved band with a name actually registered -- an unnamed one degrades to "Unknown Error" in every later stack trace, which nothing else would notice. It exercises the new ownership enforcement too, so the "reserves nothing" assertion cannot pass vacuously. ctest 13/13, ASan+UBSan 13/13, coverage 15/15 at 90/40, mutation 89.6% (155/173, unchanged). Also verified out of tree: the #error fires as the first diagnostic against a stale akerror.h, pkg-config refuses akerror 0.9.0, and an external find_package(akstdlib) consumer builds and runs against a temp-prefix install. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
611 lines
37 KiB
Markdown
611 lines
37 KiB
Markdown
# TODO
|
||
|
||
Working notes for `libakstdlib` — the akerror-wrapped libc surface.
|
||
|
||
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`).
|
||
|
||
Items marked **[CONFIRMED]** were reproduced by building the library and running a probe
|
||
program against it, not just read off the source.
|
||
|
||
---
|
||
|
||
## 1. Unit tests that should be written
|
||
|
||
### 1.0 Test-harness prerequisites — **DONE** (branch `feature/test-harness`)
|
||
|
||
`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.
|
||
|
||
- [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:
|
||
|
||
| 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 |
|
||
|
||
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.
|
||
|
||
### 1.1 Memory wrappers
|
||
|
||
- [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 File I/O
|
||
|
||
Covered by `tests/test_stream.c`.
|
||
|
||
- [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.
|
||
|
||
### 1.3 Formatted output
|
||
|
||
Covered by `tests/test_format.c`.
|
||
|
||
- [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.
|
||
|
||
### 1.4 String → number
|
||
|
||
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.
|
||
|
||
- [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.
|
||
|
||
### 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.
|
||
|
||
---
|
||
|
||
## 2. Corner cases in existing functionality that should be resolved
|
||
|
||
### 2.1 Confirmed defects (reproduced against the built library)
|
||
|
||
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`.
|
||
|
||
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.
|
||
|
||
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`.
|
||
|
||
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.
|
||
|
||
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.
|
||
|
||
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.
|
||
|
||
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.
|
||
|
||
### 2.2 Latent issues and API contract gaps
|
||
|
||
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. **`aksl_fopen` does not validate `pathname` or `mode`.** `fopen(NULL, ...)` is UB. Add
|
||
`AKERR_NULLPOINTER` guards.
|
||
|
||
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.
|
||
|
||
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.
|
||
|
||
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; no version macros
|
||
(`AKSL_VERSION_MAJOR`…); `akstdlib.h` pulls in `stdio.h`/`stdlib.h`/`string.h`/`stdint.h`
|
||
into every consumer's namespace.
|
||
|
||
17. **Doxygen coverage is 2 functions out of 20.** 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).
|
||
|
||
### 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.
|
||
- [ ] **`project()` declares no `VERSION`**, so `@PROJECT_VERSION@` expands to nothing and
|
||
the installed `akstdlib.pc` ships an empty `Version:` field — a consumer cannot pin
|
||
this library the way `akstdlib.pc` now pins `akerror >= 1.0.0`. libakstdlib also has
|
||
no `SOVERSION`, so `libakstdlib.so` carries no soname even though its public header
|
||
re-exports libakerror's ABI and therefore breaks whenever libakerror's does. Give the
|
||
project a version and a `SOVERSION`, and install an `akstdlibConfigVersion.cmake` via
|
||
`write_basic_package_version_file()` so `find_package(akstdlib 1.2 REQUIRED)` can work.
|
||
(libakerror has the same `ConfigVersion` gap, which is why
|
||
`cmake/akstdlib.cmake.in` calls `find_dependency(akerror)` with no version.)
|
||
- [ ] **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`.
|
||
|
||
---
|
||
|
||
## 3. libc functions not yet wrapped
|
||
|
||
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`".
|
||
|
||
### 3.1 High priority — gaps in areas the library already covers
|
||
|
||
**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.
|
||
|
||
**String → number (`stdlib.h`)**
|
||
- [ ] `strtol`, `strtoll`, `strtoul`, `strtoull`, `strtod`, `strtof`, `strtold` — the correct
|
||
foundation for §2.1.5.
|
||
|
||
**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`
|
||
|
||
**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)
|
||
|
||
**`unistd.h` / `fcntl.h`**
|
||
- [ ] `open`, `close`, `read`, `write`, `pread`, `pwrite`, `lseek`
|
||
- [ ] `readv`, `writev`
|
||
- [ ] `dup`, `dup2`, `pipe`, `fcntl`
|
||
- [ ] `fsync`, `fdatasync`, `truncate`, `ftruncate`
|
||
- [ ] `unlink`, `link`, `symlink`, `readlink`, `rmdir`, `mkdir`
|
||
- [ ] `access`, `faccessat`, `chmod`, `fchmod`, `chown`, `fchown`, `umask`
|
||
- [ ] `chdir`, `fchdir`, `getcwd`
|
||
- [ ] `isatty`, `ttyname_r`
|
||
- [ ] `sysconf`, `pathconf`
|
||
- [ ] `sleep`, `usleep`, `nanosleep`
|
||
|
||
**`sys/stat.h`**
|
||
- [ ] `stat`, `fstat`, `lstat`, `fstatat`
|
||
- [ ] `statvfs`, `fstatvfs`
|
||
|
||
**`dirent.h`**
|
||
- [ ] `opendir`, `fdopendir`, `readdir`, `readdir_r`, `closedir`, `rewinddir`, `scandir`
|
||
|
||
**Process control**
|
||
- [ ] `fork`, `execve` / `execvp` / `execl` 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.
|
||
- [ ] `getenv`, `setenv`, `unsetenv`, `putenv`, `clearenv`
|
||
|
||
**`sys/mman.h`**
|
||
- [ ] `mmap`, `munmap`, `mprotect`, `msync`, `madvise`
|
||
|
||
### 3.3 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`
|
||
|
||
- [ ] `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`
|
||
- [ ] `rand`, `srand`, `random`, `srandom`, `getrandom`/`arc4random`
|
||
|
||
### 3.5 Lower priority / larger projects
|
||
|
||
- [ ] **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`
|
||
- [ ] **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`.
|
||
|
||
### 3.6 Non-libc additions the current data structures imply
|
||
|
||
Not libc wrappers, but the existing list/tree API is visibly incomplete:
|
||
|
||
- [ ] 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.
|