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>
330 lines
16 KiB
Markdown
330 lines
16 KiB
Markdown
# TODO
|
|
|
|
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.
|
|
|
|
Ordered by blast radius: what blocks other work first, what is merely wrong
|
|
second, what is missing last.
|
|
|
|
## 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. Blocked on libakerror
|
|
|
|
These are not fixable from inside this repository. Each one currently costs
|
|
something here, and the cost is what makes them worth carrying.
|
|
|
|
### 1.1 The error pool is a process-global array with no locking
|
|
|
|
**This is what makes the library single-threaded**, and it is the largest open
|
|
item by some distance.
|
|
|
|
`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.
|
|
|
|
**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.
|
|
|
|
**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.
|
|
|
|
### 1.2 `IGNORE()` logs a context and never releases it
|
|
|
|
`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)`.
|
|
|
|
**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.
|
|
|
|
**What closing it would touch.** One `RELEASE_ERROR` in the macro; then delete
|
|
the workaround here.
|
|
|
|
### 1.3 libakerror does not namespace its `coverage` target when embedded
|
|
|
|
`deps/libakerror/CMakeLists.txt:172` versus `:189` — it namespaces `mutation` and
|
|
not `coverage`.
|
|
|
|
**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`.
|
|
|
|
**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.
|
|
|
|
### 1.4 libakerror installs no `akerrorConfigVersion.cmake`
|
|
|
|
**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.
|
|
|
|
**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. Known gaps in what is already wrapped
|
|
|
|
### 2.1 A short transfer with neither EOF nor a stream error is untested
|
|
|
|
`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.
|
|
|
|
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.
|
|
|
|
**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.
|
|
|
|
**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.
|
|
|
|
### 2.2 The string-buffer overflow guard is untestable
|
|
|
|
`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.
|
|
|
|
**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.
|
|
|
|
**What closing it would touch.** Nothing worth doing. Recorded so the coverage
|
|
listing does not read as an oversight.
|
|
|
|
### 2.3 `aksl_version_check()` ignores its `patch` argument
|
|
|
|
`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.
|
|
|
|
**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.
|
|
|
|
### 2.4 Four uncovered `HANDLE(e, AKERR_ITERATOR_BREAK)` lines
|
|
|
|
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. Deliberate omissions
|
|
|
|
Recorded so nobody adds them thinking they were forgotten. Each is a decision,
|
|
and each can be revisited with an argument.
|
|
|
|
| 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. |
|
|
|
|
---
|
|
|
|
## 4. Not yet wrapped
|
|
|
|
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.
|
|
|
|
### 4.1 POSIX file and process API
|
|
|
|
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`
|
|
- [ ] `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`
|
|
|
|
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`, `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`, 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 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`
|
|
|
|
### 4.2 Time
|
|
|
|
- [ ] `time`, `clock_gettime`, `clock_getres`, `gettimeofday`
|
|
- [ ] `localtime_r`, `gmtime_r`, `mktime`, `timegm`, `difftime`
|
|
- [ ] `strftime`, `strptime`
|
|
- [ ] `clock`, `times`
|
|
|
|
`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.
|
|
|
|
### 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`
|
|
|
|
### 4.4 Larger surfaces
|
|
|
|
- [ ] **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` 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.
|
|
|
|
### 4.5 Data structures
|
|
|
|
The list and tree API is complete against what §3.6 asked for. What a second
|
|
consumer might want next:
|
|
|
|
- [ ] 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.
|
|
|
|
---
|
|
|
|
## 5. 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 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.
|
|
|
|
**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 | Count | Now available as |
|
|
|---|---|---|
|
|
| `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` |
|
|
|
|
**All four things the port had to write for itself now exist here.**
|
|
|
|
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`.
|
|
|
|
**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.
|
|
|
|
**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.
|