aksl_vsnprintf discards the required length when it raises AKERR_OUTOFBOUNDS #34

New Issue

tachikoma · 2026-08-03T12:50:16-04:00

tachikoma commented

2026-08-03 12:50:16 -04:00

Found by the akbasic port (#26).

Status: scope cut by review. Andrew reviewed the original filing (comment below) and rejected everything in it except one defect. What follows is the plan as it now stands. The rejected reasoning is kept at the bottom, with the reason it was rejected, so nobody re-derives it.

The one accepted defect

aksl_vsnprintf (src/stdlib.c:491) assigns *count after it raises AKERR_OUTOFBOUNDS. On the truncation path the caller is told the output did not fit and is never told how much room it needed -- even though vsnprintf has already returned exactly that number and the function then throws it away.

    needed = vsnprintf(str, size, format, args);
    FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error");
    FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS,
			"output truncated: %d bytes needed, %zu available", needed, size);
    *count = needed;

The required length ends up in the error message and nowhere a program can read it. That is the whole bug.

The fix

Assign *count directly from vsnprintf and get rid of the needed local variable:

    errno = 0;
    *count = vsnprintf(str, size, format, args);
    FAIL_NONZERO_RETURN(e, (*count < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error");
    FAIL_NONZERO_RETURN(e, ((size_t)*count >= size), AKERR_OUTOFBOUNDS,
			"output truncated: %d bytes needed, %zu available", *count, size);
    SUCCEED_RETURN(e);

Resulting contract for *count:

Outcome	`*count`
Any error condition	Length the complete output would have had, as `snprintf(3)` returns it

aksl_vasprintf already assigns *count last with no failure path after it, and has no truncation case at all. No change there.

What has to move with the one-line change

The "*count is 0 on every failure" invariant is written down in seven places and stops being true for AKERR_OUTOFBOUNDS. All of these need to be checked that they are checking ONLY the akerr_ErrorContext * for failure data, NOT the count variable:

src/stdlib.c:419-422
src/stdlib.c:483-490 -- the block comment above aksl_vsnprintf; it should now say the required length is handed back, and that truncation is still an error.
include/akstdlib.h:445-447 -- the Formatted-output group comment, "never the length the output would have been."
include/akstdlib.h:484 -- aksl_snprintf @param[out] count.
include/akstdlib.h:552 -- aksl_vsnprintf @param[out] count.
UPGRADING.md:89 -- "*count is 0 on any failure rather than vsprintf's -1."
README.md:39 -- the deviations table entry; truncation is still AKERR_OUTOFBOUNDS, and the entry should note the needed length now comes back in *count.

Tests

tests/test_format.c:71 test_snprintf_truncation_is_an_error -- asserts count == 0; it becomes 28, the length of "far too long for eight bytes". Its comment says "*count is 0 rather than the would-have-been length" and needs to say the opposite.
tests/test_format.c:84 test_snprintf_boundary_is_exact -- the one-byte-over case asserts count == 0; it becomes 8.
Add to the truncation test: assert buf holds "far too", the seven bytes that did fit. That the partial render survives is the property the review turns on, and nothing currently pins it.
tests/test_pool.c:119 and :157 check status only. No change.

Acceptance criteria

On truncation, aksl_snprintf/aksl_vsnprintf still raise AKERR_OUTOFBOUNDS, and *count holds the length the complete output would have had.
On ALL error conditions, *count holds the number of bytes written.
The destination still holds the truncated, NUL-terminated prefix, as it always has.
Header, source comments, README.md and UPGRADING.md all state the new *count contract, and none of them still claims it is 0 on every failure.
No signature change, no new entry point, no behavioural change to aksl_asprintf/aksl_vasprintf.

Rejected on review, with the reasoning

Recorded so it is not re-derived from the same evidence later.

"aksl_snprintf makes truncation an error and writes nothing. No opt-out." -- False. vsnprintf runs before the check, so the buffer already holds as much as fit and is NUL-terminated; only the status says otherwise. A caller who wants to render as much as fits gets exactly that today.
"akbasic's PRINT USING renders ~310 bytes into a 256-byte buffer." -- That program controls both the buffer size and the precision and is expected to know both. Not a defect in this library.
"aksl_asprintf allocates, which is the wrong shape for a caller-owned buffer." -- Allocating is asprintf(3)'s documented behaviour. Working as intended.
The proposed aksl_snprintf_trunc entry point, its *truncated out-parameter, and legalising size == 0 as a "how long would this be" query -- dropped. With 1-3 rejected there is no gap left for it to fill. "Truncation is an error" remains a principle rather than a default, so the README.md "Where it deviates from libc, and why" entry stands as written and no "Deliberate omissions" entry is needed.

The akbasic call sites listed in the original filing are akbasic's to resolve against the existing API, in that repo, not this one.

Filed by Tachikoma (Claude Code, Opus 5, 1M context)

Found by the akbasic port (#26). **Status: scope cut by review.** Andrew reviewed the original filing (comment below) and rejected everything in it except one defect. What follows is the plan as it now stands. The rejected reasoning is kept at the bottom, with the reason it was rejected, so nobody re-derives it. --- ## The one accepted defect `aksl_vsnprintf` (`src/stdlib.c:491`) assigns `*count` **after** it raises `AKERR_OUTOFBOUNDS`. On the truncation path the caller is told the output did not fit and is never told how much room it needed -- even though `vsnprintf` has already returned exactly that number and the function then throws it away. ```c needed = vsnprintf(str, size, format, args); FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error"); FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS, "output truncated: %d bytes needed, %zu available", needed, size); *count = needed; ``` The required length ends up in the error *message* and nowhere a program can read it. That is the whole bug. ## The fix Assign `*count` directly from vsnprintf and get rid of the `needed` local variable: ```c errno = 0; *count = vsnprintf(str, size, format, args); FAIL_NONZERO_RETURN(e, (*count < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error"); FAIL_NONZERO_RETURN(e, ((size_t)*count >= size), AKERR_OUTOFBOUNDS, "output truncated: %d bytes needed, %zu available", *count, size); SUCCEED_RETURN(e); ``` **Resulting contract for `*count`:** | Outcome | `*count` | |---|---| | Any error condition | Length the complete output would have had, as `snprintf(3)` returns it | `aksl_vasprintf` already assigns `*count` last with no failure path after it, and has no truncation case at all. No change there. ## What has to move with the one-line change The "`*count` is 0 on every failure" invariant is written down in seven places and stops being true for `AKERR_OUTOFBOUNDS`. All of these need to be checked that they are checking ONLY the `akerr_ErrorContext *` for failure data, NOT the count variable: - `src/stdlib.c:419-422` - `src/stdlib.c:483-490` -- the block comment above `aksl_vsnprintf`; it should now say the required length *is* handed back, and that truncation is still an error. - `include/akstdlib.h:445-447` -- the Formatted-output group comment, "never the length the output *would* have been." - `include/akstdlib.h:484` -- `aksl_snprintf` `@param[out] count`. - `include/akstdlib.h:552` -- `aksl_vsnprintf` `@param[out] count`. - `UPGRADING.md:89` -- "`*count` is `0` on any failure rather than `vsprintf`'s `-1`." - `README.md:39` -- the deviations table entry; truncation is still `AKERR_OUTOFBOUNDS`, and the entry should note the needed length now comes back in `*count`. ## Tests - `tests/test_format.c:71` `test_snprintf_truncation_is_an_error` -- asserts `count == 0`; it becomes `28`, the length of `"far too long for eight bytes"`. Its comment says "`*count` is 0 rather than the would-have-been length" and needs to say the opposite. - `tests/test_format.c:84` `test_snprintf_boundary_is_exact` -- the one-byte-over case asserts `count == 0`; it becomes `8`. - Add to the truncation test: assert `buf` holds `"far too"`, the seven bytes that did fit. That the partial render survives is the property the review turns on, and nothing currently pins it. - `tests/test_pool.c:119` and `:157` check status only. No change. ## Acceptance criteria - On truncation, `aksl_snprintf`/`aksl_vsnprintf` still raise `AKERR_OUTOFBOUNDS`, and `*count` holds the length the complete output would have had. - On ALL error conditions, *count holds the number of bytes written. - The destination still holds the truncated, NUL-terminated prefix, as it always has. - Header, source comments, `README.md` and `UPGRADING.md` all state the new `*count` contract, and none of them still claims it is 0 on every failure. - No signature change, no new entry point, no behavioural change to `aksl_asprintf`/`aksl_vasprintf`. --- ## Rejected on review, with the reasoning Recorded so it is not re-derived from the same evidence later. 1. **"`aksl_snprintf` makes truncation an error and *writes nothing*. No opt-out."** -- False. `vsnprintf` runs *before* the check, so the buffer already holds as much as fit and is NUL-terminated; only the status says otherwise. A caller who wants to render as much as fits gets exactly that today. 2. **"akbasic's `PRINT USING` renders ~310 bytes into a 256-byte buffer."** -- That program controls both the buffer size and the precision and is expected to know both. Not a defect in this library. 3. **"`aksl_asprintf` allocates, which is the wrong shape for a caller-owned buffer."** -- Allocating is `asprintf(3)`'s documented behaviour. Working as intended. 4. **The proposed `aksl_snprintf_trunc` entry point**, its `*truncated` out-parameter, and legalising `size == 0` as a "how long would this be" query -- **dropped**. With 1-3 rejected there is no gap left for it to fill. "Truncation is an error" remains a principle rather than a default, so the `README.md` "Where it deviates from libc, and why" entry stands as written and no "Deliberate omissions" entry is needed. The akbasic call sites listed in the original filing are akbasic's to resolve against the existing API, in that repo, not this one. Filed by Tachikoma (Claude Code, Opus 5, 1M context)

tachikoma added this to the 0.3.0 milestone 2026-08-03 12:50:16 -04:00

tachikoma added the api-gap design-decision blast-radius:medium status::grooming labels 2026-08-03 12:50:16 -04:00

tachikoma referenced this issue

2026-08-03 12:54:57 -04:00

Port akbasic onto this release and count the calls again #26

andrew commented

2026-08-03 14:48:36 -04:00

@tachikoma

aksl_snprintf makes truncation AKERR_OUTOFBOUNDS and writes nothing. No opt-out.

Really?

    needed = vsnprintf(str, size, format, args);
    FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error");
    FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS,
			"output truncated: %d bytes needed, %zu available", needed, size);

The AKERR_OUTOFBOUNDS exception is after the vsnprintf call. A caller who wants to render as much as fits gets exactly what they want.

BASIC's PRINT USING renders a double with %.*f into a 256-byte buffer; a large value needs ~310.

And this is our problem, how? That program is in control of both of those quantities and the author knows (or should know) both of them.

aksl_asprintf allocates, which is the wrong shape when the destination is a fixed buffer the caller already owns.

allocating is the documented behavior for asprintf in libc, this is not a bug

... who wants the required length as a value rather than as an error ...

This is the ONLY bug I'll concede here: we raise the exception before we store the number of bytes written. This is a simple fix in src/stdlib.c.

    needed = vsnprintf(str, size, format, args);
    FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error");
    FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS,
			"output truncated: %d bytes needed, %zu available", needed, size);
    *count = needed;

Move the *count = needed assignment up above the FAIL_NONZERO_RETURN statements. Or just get rid of the needed intermediate variable entirely and assign directly to *count, which would have prevented this from ever happening in the first place.

@tachikoma update the plan.

@tachikoma > aksl_snprintf makes truncation AKERR_OUTOFBOUNDS and writes nothing. No opt-out. Really? ```c needed = vsnprintf(str, size, format, args); FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error"); FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS, "output truncated: %d bytes needed, %zu available", needed, size); ``` The AKERR_OUTOFBOUNDS exception is **after** the vsnprintf call. A caller who wants to render as much as fits gets exactly what they want. > BASIC's PRINT USING renders a double with %.*f into a 256-byte buffer; a large value needs ~310. And this is our problem, how? That program is in control of both of those quantities and the author knows (or should know) both of them. > aksl_asprintf allocates, which is the wrong shape when the destination is a fixed buffer the caller already owns. allocating is the documented behavior for asprintf in libc, this is not a bug > ... who wants the required length as a value rather than as an error ... This is the ONLY bug I'll concede here: we raise the exception before we store the number of bytes written. This is a simple fix in src/stdlib.c. ```c needed = vsnprintf(str, size, format, args); FAIL_NONZERO_RETURN(e, (needed < 0), AKSL_ERRNO_OR(AKERR_IO), "Output error"); FAIL_NONZERO_RETURN(e, ((size_t)needed >= size), AKERR_OUTOFBOUNDS, "output truncated: %d bytes needed, %zu available", needed, size); *count = needed; ``` Move the `*count = needed` assignment up above the FAIL_NONZERO_RETURN statements. Or just get rid of the `needed` intermediate variable entirely and assign directly to `*count`, which would have prevented this from ever happening in the first place. @tachikoma update the plan.

👀 1

tachikoma changed title from ~~No truncating format, and no way to ask how long the result would have been~~ to aksl_vsnprintf discards the required length when it raises AKERR_OUTOFBOUNDS

2026-08-03 15:02:09 -04:00

tachikoma commented

2026-08-03 15:02:26 -04:00

Plan rewritten to match your ruling. Scope is now the one conceded bug in aksl_vsnprintf and nothing else; aksl_snprintf_trunc, the *truncated parameter and the size == 0 length query are all dropped, and the four rejected points are recorded at the bottom of the body with your reasoning so they do not get re-derived. Retitled to match. No code touched.

One thing worth a look before I execute it. Both forms you suggested -- moving *count = needed above the FAIL_NONZERO_RETURN statements, or dropping needed and assigning straight to *count -- put the assignment above both checks, which leaves *count holding vsnprintf's -1 when the output-error branch fires. That is the regression src/stdlib.c:419-422 and include/akstdlib.h:445-447 both record as fixed. Putting it between the two checks gets the truncation length back to the caller and keeps *count non-negative on the error path, so that is what the plan says. Say the word if you meant the literal placement instead.

The other tail is the *count contract itself: "0 on every failure" is written down in seven places, and it stops being true for AKERR_OUTOFBOUNDS. Those are listed in the plan, along with two tests/test_format.c assertions that currently pin count == 0 on the truncation paths.

Plan rewritten to match your ruling. Scope is now the one conceded bug in `aksl_vsnprintf` and nothing else; `aksl_snprintf_trunc`, the `*truncated` parameter and the `size == 0` length query are all dropped, and the four rejected points are recorded at the bottom of the body with your reasoning so they do not get re-derived. Retitled to match. No code touched. One thing worth a look before I execute it. Both forms you suggested -- moving `*count = needed` above the `FAIL_NONZERO_RETURN` statements, or dropping `needed` and assigning straight to `*count` -- put the assignment above *both* checks, which leaves `*count` holding `vsnprintf`'s `-1` when the output-error branch fires. That is the regression `src/stdlib.c:419-422` and `include/akstdlib.h:445-447` both record as fixed. Putting it between the two checks gets the truncation length back to the caller and keeps `*count` non-negative on the error path, so that is what the plan says. Say the word if you meant the literal placement instead. The other tail is the `*count` contract itself: "0 on every failure" is written down in seven places, and it stops being true for `AKERR_OUTOFBOUNDS`. Those are listed in the plan, along with two `tests/test_format.c` assertions that currently pin `count == 0` on the truncation paths.

andrew added the status::ready label 2026-08-03 15:10:58 -04:00

logikoma was assigned by andrew

2026-08-03 15:11:02 -04:00

tachikoma referenced this issue

2026-08-03 15:37:10 -04:00

Port akbasic onto this release and count the calls again #26

tachikoma referenced this issue from a commit

2026-08-03 15:45:08 -04:00

Recount at 2.2% once the eight bool predicates convert

tachikoma referenced this issue

2026-08-03 15:46:37 -04:00

Recount the consumer calls against this release (#26) #39