Merge pull request 'Move outstanding work from TODO.md into the issue tracker' (#28) from todo-to-issues into main
All checks were successful
All checks were successful
Reviewed-on: #28
This commit is contained in:
27
AGENTS.md
27
AGENTS.md
@@ -98,8 +98,9 @@ for the ..." on their own expansion, not on anything at the call site.
|
|||||||
Add a new test by creating `tests/test_mything.c` and adding `mything` to the
|
Add a new test by creating `tests/test_mything.c` and adding `mything` to the
|
||||||
right list in `CMakeLists.txt`. `AKSL_TESTS` must exit zero.
|
right list in `CMakeLists.txt`. `AKSL_TESTS` must exit zero.
|
||||||
`AKSL_WILL_FAIL_TESTS` are deliberate abort/contract tests.
|
`AKSL_WILL_FAIL_TESTS` are deliberate abort/contract tests.
|
||||||
`AKSL_KNOWN_FAILING_TESTS` assert documented defects from `TODO.md`; when one
|
`AKSL_KNOWN_FAILING_TESTS` assert defects that have an open issue; when one
|
||||||
starts unexpectedly passing, move it into `AKSL_TESTS` with the fix. Both of the
|
starts unexpectedly passing, move it into `AKSL_TESTS` with the fix and close the
|
||||||
|
issue. Both of the
|
||||||
latter are currently empty -- all six confirmed defects are fixed -- but the
|
latter are currently empty -- all six confirmed defects are fixed -- but the
|
||||||
mechanism stays for the next one.
|
mechanism stays for the next one.
|
||||||
|
|
||||||
@@ -119,7 +120,7 @@ file, `find_dependency(akerror)` resolving, and the exported
|
|||||||
Coverage is 99.5% of lines and 100% of functions across all four sources; CI
|
Coverage is 99.5% of lines and 100% of functions across all four sources; CI
|
||||||
gates at 90 (line) / 40 (branch), so new code needs tests in the same commit. Run
|
gates at 90 (line) / 40 (branch), so new code needs tests in the same commit. Run
|
||||||
`cmake --build build-coverage --target coverage` and check the uncovered-line
|
`cmake --build build-coverage --target coverage` and check the uncovered-line
|
||||||
listing before proposing a change. Tests for behaviour that `TODO.md` records as
|
listing before proposing a change. Tests for behaviour an open issue records as
|
||||||
defective belong in `AKSL_KNOWN_FAILING_TESTS` asserting the *correct* contract —
|
defective belong in `AKSL_KNOWN_FAILING_TESTS` asserting the *correct* contract —
|
||||||
do not pin current-but-wrong behaviour in `AKSL_TESTS`, since that turns the
|
do not pin current-but-wrong behaviour in `AKSL_TESTS`, since that turns the
|
||||||
eventual fix into a test failure.
|
eventual fix into a test failure.
|
||||||
@@ -130,10 +131,26 @@ Recent commits use short imperative summaries, for example `Add memory wrapper
|
|||||||
tests` and `Make error-status assertions authoritative`. Keep commits focused
|
tests` and `Make error-status assertions authoritative`. Keep commits focused
|
||||||
and include tests with behavior changes. Pull requests should describe the
|
and include tests with behavior changes. Pull requests should describe the
|
||||||
changed API or behavior, list the CTest/sanitizer/mutation commands run, and
|
changed API or behavior, list the CTest/sanitizer/mutation commands run, and
|
||||||
link the relevant `TODO.md` item or issue when fixing a known defect.
|
link the issue it closes.
|
||||||
|
|
||||||
## Agent-Specific Instructions
|
## Agent-Specific Instructions
|
||||||
|
|
||||||
|
**Outstanding work goes in the issue tracker, not in a file.** Open an issue at
|
||||||
|
<https://source.starfort.tech/andrew/libakstdlib/issues> — `tea issues create
|
||||||
|
--repo andrew/libakstdlib` — naming the file and line, the functional
|
||||||
|
consequence, and what closing it would touch. Label it by kind and blast radius
|
||||||
|
and leave `status::grooming` on it until its scope and approach are settled.
|
||||||
|
**Do not add outstanding items to `TODO.md`**: that file is the record of where
|
||||||
|
the library stands, what is deliberately not wrapped, and which uncovered lines
|
||||||
|
are uncoverable rather than untested. A description of work still to do goes
|
||||||
|
stale the moment somebody does it, which is why the two are separated.
|
||||||
|
|
||||||
|
**A defect in a dependency is filed against that dependency.** `libakerror` has
|
||||||
|
a tracker on the same forge, and three defects that cost this library a
|
||||||
|
workaround each sat in this repository's own notes for months without anybody
|
||||||
|
upstream being able to see them. Comment the workaround at its site with the
|
||||||
|
words "filed upstream" and delete it when the fix lands.
|
||||||
|
|
||||||
Do not modify generated build trees, profiling artifacts, or untracked scratch
|
Do not modify generated build trees, profiling artifacts, or untracked scratch
|
||||||
files unless explicitly asked. Prefer small, test-backed changes and update
|
files unless explicitly asked. Prefer small, test-backed changes and update
|
||||||
`README.md` or `TODO.md` when changing documented workflows or known failures.
|
`README.md` when changing documented workflows.
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ endif()
|
|||||||
# Sanitizer build, off by default:
|
# Sanitizer build, off by default:
|
||||||
# cmake -S . -B build-asan -DAKSL_SANITIZE=ON && ctest --test-dir build-asan
|
# cmake -S . -B build-asan -DAKSL_SANITIZE=ON && ctest --test-dir build-asan
|
||||||
# Set before the dependency is added so libakerror is instrumented too --
|
# Set before the dependency is added so libakerror is instrumented too --
|
||||||
# several of the defects in TODO.md section 2 (the uninitialised %s in
|
# several of the defects listed in UPGRADING.md (the uninitialised %s in
|
||||||
# aksl_realpath, the unbounded vsprintf in aksl_sprintf, the missing va_end in
|
# aksl_realpath, the unbounded vsprintf in aksl_sprintf, the missing va_end in
|
||||||
# the printf family) only show up under ASan/UBSan.
|
# the printf family) only show up under ASan/UBSan.
|
||||||
option(AKSL_SANITIZE "Build the library and its tests with ASan + UBSan" OFF)
|
option(AKSL_SANITIZE "Build the library and its tests with ASan + UBSan" OFF)
|
||||||
@@ -298,7 +298,7 @@ install(FILES
|
|||||||
# reaching FINISH_NORETURN, a deliberate contract
|
# reaching FINISH_NORETURN, a deliberate contract
|
||||||
# violation), so a non-zero exit is a pass.
|
# violation), so a non-zero exit is a pass.
|
||||||
# AKSL_KNOWN_FAILING_TESTS assert the *correct* behaviour of a confirmed
|
# AKSL_KNOWN_FAILING_TESTS assert the *correct* behaviour of a confirmed
|
||||||
# defect from TODO.md section 2.1. They fail until
|
# defect; see UPGRADING.md. They fail until
|
||||||
# the defect is fixed, and are marked WILL_FAIL so
|
# the defect is fixed, and are marked WILL_FAIL so
|
||||||
# the suite stays green and the gap stays visible.
|
# the suite stays green and the gap stays visible.
|
||||||
# When one is fixed CTest reports it as failed with
|
# When one is fixed CTest reports it as failed with
|
||||||
|
|||||||
12
README.md
12
README.md
@@ -56,7 +56,7 @@ error reporting is not.
|
|||||||
|
|
||||||
There is no TSan test here because there is nothing to verify — the answer is
|
There is no TSan test here because there is nothing to verify — the answer is
|
||||||
known and it is "no". Fixing it means locking or thread-local storage in
|
known and it is "no". Fixing it means locking or thread-local storage in
|
||||||
libakerror's pool, which is that library's decision to make; `TODO.md` §1.9
|
libakerror's pool, which is that library's decision to make; issue #2
|
||||||
records it. Until then: confine libakstdlib calls to one thread, or serialise
|
records it. Until then: confine libakstdlib calls to one thread, or serialise
|
||||||
them yourself.
|
them yourself.
|
||||||
|
|
||||||
@@ -100,7 +100,7 @@ copy in your build directory; change the template or `project()`.
|
|||||||
It is `0.x` deliberately. The 0.1 → 0.2 bump was itself an ABI break — fixing the
|
It is `0.x` deliberately. The 0.1 → 0.2 bump was itself an ABI break — fixing the
|
||||||
confirmed defects changed five signatures and the `ato*` contract, all of it
|
confirmed defects changed five signatures and the `ato*` contract, all of it
|
||||||
listed in `UPGRADING.md` — and the API is not being promised until the wishlist
|
listed in `UPGRADING.md` — and the API is not being promised until the wishlist
|
||||||
in `TODO.md` §3 has settled. While the major version is `0`, **the soname carries
|
in the tracker has settled. While the major version is `0`, **the soname carries
|
||||||
`MAJOR.MINOR`**: 0.1 and 0.2
|
`MAJOR.MINOR`**: 0.1 and 0.2
|
||||||
are different ABIs and the loader will not substitute one for the other. At 1.0
|
are different ABIs and the loader will not substitute one for the other. At 1.0
|
||||||
the soname becomes `MAJOR` alone — the `if(PROJECT_VERSION_MAJOR EQUAL 0)` in
|
the soname becomes `MAJOR` alone — the `if(PROJECT_VERSION_MAJOR EQUAL 0)` in
|
||||||
@@ -225,10 +225,10 @@ of them invert the meaning of "Passed":
|
|||||||
|---|---|
|
|---|---|
|
||||||
| `AKSL_TESTS` | Ordinary tests. Must exit 0. |
|
| `AKSL_TESTS` | Ordinary tests. Must exit 0. |
|
||||||
| `AKSL_WILL_FAIL_TESTS` | Expected to abort by design — an unhandled error reaching `FINISH_NORETURN`, or a deliberate contract violation. Marked `WILL_FAIL`, so a non-zero exit is a pass. |
|
| `AKSL_WILL_FAIL_TESTS` | Expected to abort by design — an unhandled error reaching `FINISH_NORETURN`, or a deliberate contract violation. Marked `WILL_FAIL`, so a non-zero exit is a pass. |
|
||||||
| `AKSL_KNOWN_FAILING_TESTS` | Assert the *correct* behaviour of a confirmed defect (see `TODO.md` §2.1). Also marked `WILL_FAIL`. |
|
| `AKSL_KNOWN_FAILING_TESTS` | Assert the *correct* behaviour of a confirmed defect (see `UPGRADING.md`). Also marked `WILL_FAIL`. |
|
||||||
|
|
||||||
**Both of those lists are currently empty**, which is the news: all six confirmed
|
**Both of those lists are currently empty**, which is the news: all six confirmed
|
||||||
defects in `TODO.md` §2.1 are fixed, and the four tests that used to sit in
|
defects recorded in `UPGRADING.md` are fixed, and the four tests that used to sit in
|
||||||
`AKSL_KNOWN_FAILING_TESTS` are folded back into the tests for the things they
|
`AKSL_KNOWN_FAILING_TESTS` are folded back into the tests for the things they
|
||||||
test, where they now have to keep passing rather than keep failing visibly. The
|
test, where they now have to keep passing rather than keep failing visibly. The
|
||||||
mechanism stays for the next one. When a defect is fixed its known-failing test
|
mechanism stays for the next one. When a defect is fixed its known-failing test
|
||||||
@@ -322,7 +322,7 @@ so a top-level `-DAKSL_COVERAGE=ON` build would collide on the name and fail to
|
|||||||
configure at all. `CMakeLists.txt` renames the dependency's to `akerror_coverage`
|
configure at all. `CMakeLists.txt` renames the dependency's to `akerror_coverage`
|
||||||
on the way past — it drives its own instrumented build tree, so
|
on the way past — it drives its own instrumented build tree, so
|
||||||
`cmake --build build-coverage --target akerror_coverage` still works. The
|
`cmake --build build-coverage --target akerror_coverage` still works. The
|
||||||
workaround goes away when libakerror namespaces it upstream; see `TODO.md` §2.3.
|
workaround goes away when libakerror namespaces it upstream; see issue #4.
|
||||||
|
|
||||||
CTest hides the output of a passing test, so `coverage_report` also writes
|
CTest hides the output of a passing test, so `coverage_report` also writes
|
||||||
`build-coverage/coverage-summary.txt` (the same text report) and
|
`build-coverage/coverage-summary.txt` (the same text report) and
|
||||||
@@ -379,7 +379,7 @@ lines are uncovered and each is uncovered on purpose:
|
|||||||
- **Two in `aksl_fread`/`aksl_fwrite`**, the short transfer with *neither* `feof`
|
- **Two in `aksl_fread`/`aksl_fwrite`**, the short transfer with *neither* `feof`
|
||||||
nor `ferror` set. Every way of producing a short transfer on Linux sets one or
|
nor `ferror` set. Every way of producing a short transfer on Linux sets one or
|
||||||
the other; the branch is there because the standard permits neither, not
|
the other; the branch is there because the standard permits neither, not
|
||||||
because anything reaches it. `TODO.md` §1.2 records it as still open.
|
because anything reaches it. Issue #6 records it as still open.
|
||||||
|
|
||||||
Branch coverage sits far below line coverage because most branches in these files
|
Branch coverage sits far below line coverage because most branches in these files
|
||||||
are inside the `FAIL_*`/`ATTEMPT`/`FINISH` macro expansions — pool exhaustion,
|
are inside the `FAIL_*`/`ATTEMPT`/`FINISH` macro expansions — pool exhaustion,
|
||||||
|
|||||||
392
TODO.md
392
TODO.md
@@ -1,11 +1,16 @@
|
|||||||
# TODO
|
# Record
|
||||||
|
|
||||||
Working notes for `libakstdlib`. **Outstanding items only** — anything fixed comes
|
**Outstanding work is in the issue tracker, not in this file:**
|
||||||
out of this file and goes into `UPGRADING.md`, the tests, or a comment beside the
|
<https://source.starfort.tech/andrew/libakstdlib/issues>
|
||||||
code, whichever is the right place to be reminded of it.
|
|
||||||
|
|
||||||
Ordered by blast radius: what blocks other work first, what is merely wrong
|
What stays here is what a tracker has no place for: where the library stands, and
|
||||||
second, what is missing last.
|
the decisions that would otherwise be re-litigated — what is deliberately *not*
|
||||||
|
wrapped, and which uncovered lines are uncoverable rather than untested.
|
||||||
|
|
||||||
|
Issues are labelled by kind and blast radius, and milestoned by what they can land
|
||||||
|
in: `0.2.x` for anything that breaks no ABI, `0.3.0` for new public symbols,
|
||||||
|
`1.0.0` for the large surfaces. Everything filed carries `status::grooming` until
|
||||||
|
it has been through grooming.
|
||||||
|
|
||||||
## Where the library stands
|
## Where the library stands
|
||||||
|
|
||||||
@@ -19,168 +24,61 @@ second, what is missing last.
|
|||||||
| Mutation score | 72.3% (188/260 sampled from 1701), gated at 65 |
|
| Mutation score | 72.3% (188/260 sampled from 1701), gated at 65 |
|
||||||
|
|
||||||
The six confirmed defects that used to head this file are fixed and
|
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
|
`AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a result,
|
||||||
result, is in `UPGRADING.md`.
|
is in `UPGRADING.md`.
|
||||||
|
|
||||||
---
|
## What libakerror costs this library
|
||||||
|
|
||||||
## 1. Blocked on libakerror
|
Three of these are not fixable from inside this repository, and each costs
|
||||||
|
something here. They are filed in both places, because the fix is there and the
|
||||||
|
bill is here. The fourth turned out not to be blocked at all:
|
||||||
|
|
||||||
These are not fixable from inside this repository. Each one currently costs
|
| Here | Upstream | What it costs |
|
||||||
something here, and the cost is what makes them worth carrying.
|
|---|---|---|
|
||||||
|
| #2 | — | **Corrected while filing.** The unlocked error pool is a property of the libakerror this repository *pins* (1.0.0), not of libakerror (2.0.1, which locks it). `libakgl` and `akbasic` are both on 2.0.1. The work is a submodule bump and the verification that goes with it, not a wait |
|
||||||
|
| #3 | libakerror | `IGNORE()` leaks a context, so `aksl_tree_iterate` open-codes log-then-release in four lines that should be one |
|
||||||
|
| #4 | libakerror | The `coverage` target is not namespaced when embedded, so `-DAKSL_COVERAGE=ON` fails to configure and `CMakeLists.txt` shadows `add_custom_target` to work around it |
|
||||||
|
| #5 | libakerror | No `akerrorConfigVersion.cmake`, so `find_dependency(akerror)` cannot ask for the 1.0.0 floor |
|
||||||
|
|
||||||
### 1.1 The error pool is a process-global array with no locking
|
## Uncovered lines that are uncoverable
|
||||||
|
|
||||||
**This is what makes the library single-threaded**, and it is the largest open
|
Eight lines, and this is what they are, so the coverage listing does not read as an
|
||||||
item by some distance.
|
oversight.
|
||||||
|
|
||||||
`AKERR_ARRAY_ERROR` is a fixed array in `deps/libakerror/src/error.c`, handed out
|
**Two are the short-transfer branch** in `aksl_fread`/`aksl_fwrite` — a short
|
||||||
by `akerr_next_error()` with no synchronisation of any kind. Every entry point in
|
transfer with neither EOF nor a stream error. The standard permits it, so the
|
||||||
this library takes a slot from it on any failure path, so two threads raising
|
branch is correct to have; every way of actually producing one on Linux sets `feof`
|
||||||
errors concurrently can be handed the same slot and will corrupt each other's
|
or `ferror` first. **It is the only error path in the library that has never
|
||||||
message, status and stack trace.
|
executed**, and reaching it needs a `FILE *` over a custom stream (`fopencookie`,
|
||||||
|
`funopen`). That is #6.
|
||||||
|
|
||||||
**Consequence.** `README.md` says plainly that the library is not thread-safe.
|
**Two are the string-buffer overflow guard** — the `capacity = needed` arm in
|
||||||
That is honest, and it is also a hard ceiling: §4's `pthread_*` and socket
|
`strbuf_reserve`. Reaching it needs an `aksl_StrBuf` within a factor of two of
|
||||||
wrappers cannot be written until this is resolved, because a threading API
|
`SIZE_MAX`, **which is not a test, it is a hang.** It is there because doubling a
|
||||||
nobody can call from a thread is not an API.
|
capacity is a multiplication, and an unguarded one is how a growable buffer turns
|
||||||
|
into a heap overflow. Nothing to do.
|
||||||
|
|
||||||
**What closing it would touch.** libakerror's pool — either a mutex around
|
**Four are `HANDLE(e, AKERR_ITERATOR_BREAK)` lines**, and they are macro artifacts
|
||||||
`akerr_next_error`/`akerr_release_error`, or thread-local slot arrays, which
|
rather than gaps. In libakerror that macro begins with the `break;` belonging to
|
||||||
would suit the bounded-preallocation style better and cost nothing on the
|
`PROCESS`'s `case 0:` arm, reachable only when a callback returns a non-NULL context
|
||||||
single-threaded path. Then a TSan job in `.gitea/workflows/ci.yaml` and a
|
whose status is *zero* — the pathological case the errno-fallback work removed. Left
|
||||||
concurrent smoke test here, and the warning in `README.md` comes out.
|
uncovered deliberately rather than pinned by a test that would have to manufacture
|
||||||
|
it.
|
||||||
|
|
||||||
### 1.2 `IGNORE()` logs a context and never releases it
|
## `aksl_version_check()` ignores its `patch` argument
|
||||||
|
|
||||||
`deps/libakerror/include/akerror.tmpl.h:308`. The macro assigns the context to
|
`src/stdlib.c`, the `(void)patch`. **Correct for the current "same soname" rule** —
|
||||||
`__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
|
patch level never breaks the ABI — but the parameter exists only so the error
|
||||||
message can name the caller's full version.
|
message can name the caller's full version.
|
||||||
|
|
||||||
**Consequence.** None today. If a future compatibility rule needs the patch level
|
No consequence today. If a future compatibility rule needs the patch level to
|
||||||
to participate, that is the line to change, and the `#if` in
|
participate, that is the line to change, and the `#if` in `tests/test_version.c` is
|
||||||
`tests/test_version.c` is the test that encodes the rule.
|
the test that encodes the rule.
|
||||||
|
|
||||||
### 2.4 Surviving mutants worth turning into assertions
|
## Deliberate omissions
|
||||||
|
|
||||||
The mutation harness samples 260 of 1701 mutants and kills 72.3% of them. Most of
|
Recorded so nobody adds them thinking they were forgotten. **Each is a decision,
|
||||||
the 72 survivors are equivalent mutants rather than missing tests — `README.md`
|
and each can be revisited with an argument.**
|
||||||
has the full breakdown — but three clusters are real work:
|
|
||||||
|
|
||||||
- **`FINISH(e, true)` → `FINISH(e, false)`, 3 survivors.** An error swallowed
|
|
||||||
instead of propagated out of an `ATTEMPT` block, and nothing notices. Each one
|
|
||||||
is a call whose failure path is exercised but whose *propagation* is not: the
|
|
||||||
test asserts the status the callee raised without checking it came from the
|
|
||||||
callee rather than being re-raised locally. `tests/test_pool.c`'s origin
|
|
||||||
assertions are the shape of the fix.
|
|
||||||
- **`SUCCEED_RETURN` deleted, 7 survivors.** The function falls off the end and
|
|
||||||
returns whatever is in the return register, which is NULL often enough to pass.
|
|
||||||
These need an assertion on the *side effect* — the buffer that was filled, the
|
|
||||||
node that was linked — rather than on the returned status.
|
|
||||||
- **`FAIL_*` guards deleted, ~6 of the 14 in that group.** Each is an argument
|
|
||||||
check nothing drives. The other 8 in the group are constant shifts that no test
|
|
||||||
can catch, because the test names the same constant symbolically and moves with
|
|
||||||
it.
|
|
||||||
|
|
||||||
Two survivors in this class were real and are fixed: the right child's
|
|
||||||
`depth + 1` in the depth-first walk, and `aksl_tree_remove` on an empty tree.
|
|
||||||
|
|
||||||
**What closing them would touch.** Only `tests/`. Raise the `--threshold` in
|
|
||||||
`.gitea/workflows/ci.yaml` and `.githooks/pre-push` in step, as a ratchet.
|
|
||||||
|
|
||||||
### 2.5 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 |
|
| Not wrapped | Why |
|
||||||
|---|---|
|
|---|---|
|
||||||
@@ -191,126 +89,23 @@ and each can be revisited with an argument.
|
|||||||
| `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. |
|
| `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. |
|
| `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. |
|
||||||
|
|
||||||
---
|
## What the mutation survivors mean
|
||||||
|
|
||||||
## 4. Not yet wrapped
|
The harness samples 260 of 1701 mutants and kills 72.3%. **Most of the 72 survivors
|
||||||
|
are equivalent mutants rather than missing tests** — `README.md` has the full
|
||||||
|
breakdown — and knowing which is which is the point, because a ratchet built on the
|
||||||
|
wrong number is a ratchet that stops moving.
|
||||||
|
|
||||||
Ordered by how much a caller of this library would miss them. §3.1 and §3.6 of
|
Three clusters are real work and are #7. Two survivors in that class were real and
|
||||||
the old numbering are done; what follows is what is left.
|
are fixed: the right child's `depth + 1` in the depth-first walk, and
|
||||||
|
`aksl_tree_remove` on an empty tree.
|
||||||
|
|
||||||
### 4.1 POSIX file and process API
|
## Evidence from the first full consumer
|
||||||
|
|
||||||
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
|
`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
|
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
|
whole surface rather than a corner of it, **and what it could not use is what
|
||||||
prioritised the work above.
|
prioritised everything that has been built since.**
|
||||||
|
|
||||||
**The number that started it.** Across `src/`, akbasic made **10 calls into this
|
**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
|
library and 116 to raw libc** — a library whose value proposition is "turn silent
|
||||||
@@ -330,48 +125,27 @@ committed to it.
|
|||||||
|
|
||||||
**All four things the port had to write for itself now exist here.**
|
**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).
|
1. **A strict `strtoll`/`strtod` wrapper** (`akbasic/src/convert.c`, ~60 lines). The
|
||||||
The `aksl_strto*` family is that, with the endptr/`errno`/range contract.
|
`aksl_strto*` family is that, with the endptr/`errno`/range contract. akbasic
|
||||||
akbasic's own `TODO.md` §1.9 formally bans the `aksl_ato*` family because
|
formally banned the `aksl_ato*` family because routing four diagnosable errors
|
||||||
routing four diagnosable errors through it would have turned them into wrong
|
through it would have turned them into wrong answers — `VAL("garbage")` silently
|
||||||
answers — `VAL("garbage")` silently returning `0.0`. That ban can be lifted:
|
returning `0.0`. **That ban can be lifted**: the `ato*` forms report failures now.
|
||||||
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,
|
||||||
2. **A fixed-capacity string-keyed hash table** (`akbasic/src/symtab.c`, ~130
|
needed three times over). `aksl_hashmap_*` is that table generalised, **with
|
||||||
lines, needed three times over). `aksl_hashmap_*` is that table generalised,
|
tombstones on delete, which the original did not have.**
|
||||||
with tombstones on delete, which the original did not have.
|
3. **The bounded-copy-with-truncation-as-error idiom, at ten sites.** `aksl_strcpy`
|
||||||
3. **The bounded-copy-with-truncation-as-error idiom, at ten sites.**
|
and `aksl_strncpy` are exactly that idiom.
|
||||||
`aksl_strcpy` and `aksl_strncpy` are exactly that idiom.
|
4. **Uppercase folding for case-insensitive lookup, three times.** `aksl_strcasecmp`
|
||||||
4. **Uppercase folding for case-insensitive lookup, three times.**
|
and `aksl_strncasecmp`.
|
||||||
`aksl_strcasecmp` and `aksl_strncasecmp`.
|
|
||||||
|
|
||||||
**And the four confirmed-with-impact items are closed.** §2.2.4's unbounded
|
**And the four confirmed-with-impact defects are closed.** The unbounded
|
||||||
`aksl_sprintf` is gone; §2.2.2's unchecked `aksl_fopen` arguments are checked, so
|
`aksl_sprintf` is gone; `aksl_fopen`'s arguments are checked, so
|
||||||
`akbasic_cmd_dload`'s hand-rolled validation and its comment pointing here can go;
|
`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` —
|
the sign-extended djb2 reads bytes unsigned; and the missing `va_end` — which
|
||||||
which akbasic's stdio text sink ran on every line of program output — is fixed.
|
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
|
**Still true, and still shaping the wishlist.** akbasic uses no allocator, no lists
|
||||||
lists and no trees, drawing everything from fixed pools by design. A consumer that
|
and no trees, drawing everything from fixed pools by design. **A consumer that does
|
||||||
does allocate would weight §4.1's `open`/`read`/`write` far higher than this one
|
allocate would weight the `open`/`read`/`write` work far higher than this one
|
||||||
does. The next thing worth doing is porting akbasic onto this release and
|
does**, so one consumer's count is evidence, not a plan. Re-counting against this
|
||||||
counting the calls again.
|
release is #26.
|
||||||
|
|
||||||
## Requested by consumers
|
|
||||||
|
|
||||||
1. **No directory-reading wrapper.** There is no `aksl_opendir` / `aksl_readdir` /
|
|
||||||
`aksl_closedir`, so a consumer that wants to list a directory has to call
|
|
||||||
`opendir(3)` itself and step outside the error convention every other call in
|
|
||||||
its file follows — reporting through `errno` where everything around it
|
|
||||||
reports through an `akerr_ErrorContext *`.
|
|
||||||
|
|
||||||
`akbasic` hit this implementing Commodore BASIC's `DIRECTORY` verb, and
|
|
||||||
refuses the verb rather than working around it: `src/runtime_disk.c` reports
|
|
||||||
"DIRECTORY is not implemented: libakstdlib has no directory-reading wrapper
|
|
||||||
yet". The shape it wants is the one the `aksl_f*` family already has — a
|
|
||||||
handle out through a pointer parameter, `NULL` on success, `ENOENT` and
|
|
||||||
`EACCES` propagated as themselves.
|
|
||||||
|
|
||||||
A wrapper would want tests for: a directory that does not exist, one that
|
|
||||||
cannot be searched, an empty one, and one whose entries outlast a single
|
|
||||||
read — plus whatever the `d_type` portability story turns out to be, since
|
|
||||||
not every filesystem fills it in.
|
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ this up. Rebuild against the new header.
|
|||||||
`AKSL_VERSION_CHECK()` catches the pairing at runtime if a stale `.so` ever does
|
`AKSL_VERSION_CHECK()` catches the pairing at runtime if a stale `.so` ever does
|
||||||
end up on the path.
|
end up on the path.
|
||||||
|
|
||||||
Everything below comes out of `TODO.md` sections 2.1 and 2.2 — the confirmed
|
Everything below was recorded in `TODO.md` before the move to the tracker — the confirmed
|
||||||
defects, all of which were reproduced against the 0.1.0 library before being
|
defects, all of which were reproduced against the 0.1.0 library before being
|
||||||
fixed.
|
fixed.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user