532 lines
31 KiB
Markdown
532 lines
31 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`. Deliberately *not* a CI gate yet, and no
|
|||
|
|
`--threshold` is set: a mutation score only means something once §1.1–1.9 exist, and
|
|||
|
|
against today's tests it would do little but confirm that most of the library is
|
|||
|
|
untested.
|
|||
|
|
- [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
|
|||
|
|
|
|||
|
|
- [ ] `aksl_malloc` — happy path returns non-NULL and writes through `dst`.
|
|||
|
|
- [ ] `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.
|
|||
|
|
- [ ] `aksl_free(NULL)` → `AKERR_NULLPOINTER` (assert the documented behaviour, since
|
|||
|
|
`free(NULL)` is legal C and callers will be surprised).
|
|||
|
|
- [ ] `aksl_free` happy path, and a malloc→free round trip under ASan.
|
|||
|
|
- [ ] `aksl_memset` — happy path fills the buffer; `n == 0` is a no-op; `s == NULL` →
|
|||
|
|
`AKERR_NULLPOINTER`.
|
|||
|
|
- [ ] `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
|
|||
|
|
|
|||
|
|
- [ ] `aksl_fopen` happy path on a temp file; `*fp` is written.
|
|||
|
|
- [ ] `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`.
|
|||
|
|
- [ ] `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.
|
|||
|
|
- [ ] `aksl_fread` full read; short read at EOF → `AKERR_EOF`; read from a write-only stream
|
|||
|
|
→ `AKERR_IO`; `fp == NULL` → `AKERR_NULLPOINTER`; `ptr == NULL` → should be
|
|||
|
|
`AKERR_NULLPOINTER` (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.
|
|||
|
|
- [ ] `aksl_fwrite` happy path; write to a read-only stream → `AKERR_IO`; write to a full
|
|||
|
|
device (`/dev/full`) → `ENOSPC`/`AKERR_IO`; `fp == NULL` → `AKERR_NULLPOINTER`.
|
|||
|
|
- [ ] `aksl_fclose` happy path; `NULL` → `AKERR_NULLPOINTER`; double-close is caught or
|
|||
|
|
documented as UB.
|
|||
|
|
- [ ] `aksl_fclose` on a stream whose buffered flush fails (`/dev/full`) → non-zero `fclose`
|
|||
|
|
surfaces `errno`.
|
|||
|
|
- [ ] Round-trip test: `fopen` → `fwrite` → `fclose` → `fopen` → `fread` → compare bytes.
|
|||
|
|
|
|||
|
|
### 1.3 Formatted output
|
|||
|
|
|
|||
|
|
- [ ] `aksl_printf` / `aksl_fprintf` / `aksl_sprintf` happy paths, asserting both the byte
|
|||
|
|
count written through `count` and the produced text.
|
|||
|
|
- [ ] Each of the three with every pointer argument NULL in turn → `AKERR_NULLPOINTER`.
|
|||
|
|
- [ ] `aksl_fprintf` to a closed / read-only stream → error path, and confirm `*count` is not
|
|||
|
|
left holding `-1` as if it were a valid length.
|
|||
|
|
- [ ] `aksl_sprintf` with a format that overflows the destination — currently unbounded
|
|||
|
|
(§2.2.4). Test the `aksl_snprintf` replacement once it exists.
|
|||
|
|
- [ ] Regression test for the missing `va_end` (§2.1.4) — a test that calls each variadic
|
|||
|
|
wrapper many times in a loop, run under valgrind/ASan.
|
|||
|
|
- [ ] 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
|
|||
|
|
|
|||
|
|
- [ ] `aksl_atoi` / `atol` / `atoll` / `atof` happy paths, including negative values and
|
|||
|
|
leading whitespace.
|
|||
|
|
- [ ] NULL `nptr` and NULL `dest` for each → `AKERR_NULLPOINTER`.
|
|||
|
|
- [ ] **Non-numeric input** (`"not a number"`) — **[CONFIRMED]** currently returns *success*
|
|||
|
|
with `*dest == 0`. Test the `AKERR_VALUE` behaviour once §2.1.5 is fixed.
|
|||
|
|
- [ ] **Overflow** (`"99999999999999999999"`) — **[CONFIRMED]** currently returns success with
|
|||
|
|
a garbage value (`-1` on this box). Test for `ERANGE`.
|
|||
|
|
- [ ] Empty string, `" "`, `"12abc"` (trailing junk), `"0x10"`, `"inf"`/`"nan"` for `atof`.
|
|||
|
|
- [ ] `LONG_MIN`/`LONG_MAX`/`LLONG_MIN`/`LLONG_MAX` boundary strings round-trip exactly.
|
|||
|
|
|
|||
|
|
### 1.5 `aksl_realpath`
|
|||
|
|
|
|||
|
|
- [ ] Happy path on an existing file and on a symlink chain; result matches `realpath(3)`.
|
|||
|
|
- [ ] Non-existent path → `ENOENT`.
|
|||
|
|
- [ ] A path component that is not a directory → `ENOTDIR`.
|
|||
|
|
- [ ] Symlink loop → `ELOOP`.
|
|||
|
|
- [ ] `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`
|
|||
|
|
|
|||
|
|
- [ ] Known-answer vectors: `djb2("")` == 5381; a handful of fixed strings with their
|
|||
|
|
pre-computed 32-bit values.
|
|||
|
|
- [ ] `len == 0` returns 5381 regardless of `str` contents.
|
|||
|
|
- [ ] 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.
|
|||
|
|
- [ ] Embedded NUL bytes are hashed (the function is length-driven, not NUL-driven).
|
|||
|
|
- [ ] 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. Break at `tree[3]` in pre-order and assert 4 visits; today it visits all 7.
|
|||
|
|
Add the equivalent for in-order and post-order.
|
|||
|
|
- [ ] `AKSL_TREE_SEARCH_BFS` and `AKSL_TREE_SEARCH_BFS_RIGHT` → `AKERR_NOT_IMPLEMENTED`
|
|||
|
|
today; replace with real order assertions once implemented (§3 / §2.2.9).
|
|||
|
|
- [ ] **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)
|
|||
|
|
|
|||
|
|
The first three 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` and
|
|||
|
|
`tests/test_tree_iterate_break.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.
|
|||
|
|
|
|||
|
|
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.
|
|||
|
|
- [ ] **The `deps/libakerror` submodule is pinned 11 commits behind `main`** (pinned at
|
|||
|
|
`4fad0ce "Add gitea workflow"`; upstream is at `4212ff0`). The pin predates the
|
|||
|
|
refcount-leak fix, the stack-trace buffer-overflow fix, the `AKERR_MAX_ERR_VALUE`
|
|||
|
|
correction and the format-string fix. Bump it.
|
|||
|
|
- [ ] **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.
|