Fix the six confirmed defects and close the API contract gaps

TODO.md section 2.1 recorded six defects reproduced against the built
library, and section 2.2 seventeen contract gaps. Both are closed. The
four tests registered in AKSL_KNOWN_FAILING_TESTS are folded back into
the tests for the things they test, and that list is now empty.

The defects:

  2.1.1  aksl_list_append conflated Floyd cycle detection with finding
         the tail, so `tail` tracked the node behind the midpoint. Any
         append to a list of 2+ nodes silently dropped everything after
         it. Two separate walks now: Floyd to prove the list is finite,
         then a plain walk to the end.
  2.1.2  aksl_list_iterate started visiting from Floyd's `slow` cursor,
         so the whole first half of the list -- head included -- was
         never passed to the callback. It starts at the head.
  2.1.3  AKERR_ITERATOR_BREAK did not stop a tree traversal: the frame
         that raised it handled it and returned success, so the parent
         carried on into the sibling subtree. The recursion is split out
         and propagates the break; only the public entry swallows it.
  2.1.4  va_end now matches every va_start on every path.
  2.1.5  The ato* family had no error channel at all. Reimplemented over
         a new strto* family with errno cleared, an endptr check and a
         range check: AKERR_VALUE for junk, ERANGE for overflow.
  2.1.6  aksl_realpath never checked resolved_path, could not be told
         the buffer size, and formatted an unspecified buffer with %s on
         its own error path. It takes a length; aksl_realpath_alloc is
         the allocating form.

The contract gaps, in brief: errno is cleared before every wrapped call
and read back through a fallback so no error can carry status 0; fopen
validates pathname and mode; fread/fwrite report the transferred count
through a required out-param and no longer call a short transfer a
success; aksl_sprintf is gone in favour of aksl_snprintf, which treats
truncation as an error; the variadic wrappers carry format attributes;
djb2 reads bytes as unsigned; tree traversal is depth- and cycle-bounded
and implements BFS, so lalloc/lfree are used rather than merely stored;
an unknown searchmode is AKERR_VALUE rather than silent success;
list_pop takes the head by reference; aksl_freep, the node initialisers
and extern "C" are new.

Build: -pg is out of the default build (it never reached the C compiler
anyway, and it is what produced the stray gmon.out), -Wall -Wextra are
in, and there is a .gitignore.

Tests: 11 binaries, all green under the normal and sanitizer builds.
Visit-order assertions replace the step counts that could not tell the
three depth-first orders apart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-31 07:14:11 -04:00
parent fd71bcc67b
commit 55eb0334c4
17 changed files with 3272 additions and 730 deletions

View File

@@ -1,15 +1,16 @@
/*
* Stream wrappers: aksl_fopen / aksl_fread / aksl_fwrite / aksl_fclose.
*
* TODO.md section 1.2. Covered here: the happy paths, the round trip, the NULL
* guards that exist, and the two error statuses the wrappers can actually
* produce today -- AKERR_EOF from a short read and AKERR_IO from a stream whose
* error indicator is set.
* TODO.md section 1.2, now complete. The happy paths, the round trip, every
* NULL guard, both stream-error statuses, the transferred-member count that
* aksl_fread and aksl_fwrite report through nmemb_out, and the three cases that
* needed a hostile file to produce: a mode-denied path (EACCES), a full device
* (/dev/full) for the short write, and a stream whose buffered flush fails at
* fclose time.
*
* Not covered, because the behaviour is a documented gap rather than a
* contract: aksl_fopen(NULL, ...) and aksl_fopen(path, NULL, ...) are unchecked
* (2.2.2), aksl_fread/aksl_fwrite never check ptr and report a short transfer
* that is neither EOF nor error as complete success (2.2.3).
* The /dev/full tests are skipped where the device is absent -- it is a Linux
* thing -- rather than failing, and say so on stderr so a skip cannot be
* mistaken for a pass.
*
* Temp files come from aksl_temp_file() and are unlinked by the test that made
* them, so a failing test leaves nothing behind but the file it was mid-way
@@ -19,6 +20,7 @@
#include "aksl_capture.h"
#include <errno.h>
#include <sys/stat.h>
static int test_fopen_writes_stream_pointer(void)
{
@@ -41,16 +43,49 @@ static int test_fopen_reports_missing_path(void)
AKSL_CHECK_STATUS_MSG_CONTAINS(
aksl_fopen("/nonexistent/aksl/stream", "r", &fp),
ENOENT, "/nonexistent/aksl/stream");
/* *fp is cleared before the call, so a failed open cannot leave garbage. */
AKSL_CHECK(fp == NULL);
return 0;
}
static int test_fopen_rejects_null_stream_out(void)
/*
* A file the caller cannot open for reading is EACCES rather than ENOENT.
* Skipped under a user who bypasses the permission bits -- root, or anything
* holding CAP_DAC_OVERRIDE -- where chmod 000 simply does not deny anything.
*/
static int test_fopen_reports_permission_denied(void)
{
char path[AKSL_TMP_MAX];
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK(chmod(path, 0000) == 0);
if ( geteuid() == 0 ) {
fprintf(stderr, " (skipped: running as root, chmod 000 denies nothing)\n");
AKSL_CHECK(unlink(path) == 0);
return 0;
}
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fopen(path, "r", &fp), EACCES, path);
AKSL_CHECK(unlink(path) == 0);
return 0;
}
static int test_fopen_rejects_null_arguments(void)
{
char path[AKSL_TMP_MAX];
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fopen(path, "r", NULL),
AKERR_NULLPOINTER, "NULL");
AKERR_NULLPOINTER, "fp=");
/* TODO.md 2.2.2: both of these used to go straight through to fopen(3). */
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fopen(NULL, "r", &fp),
AKERR_NULLPOINTER, "pathname=");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fopen(path, NULL, &fp),
AKERR_NULLPOINTER, "mode=");
AKSL_CHECK(unlink(path) == 0);
return 0;
}
@@ -61,18 +96,22 @@ static int test_write_read_round_trip(void)
char path[AKSL_TMP_MAX];
char payload[] = "libakstdlib round trip";
char readback[sizeof(payload)];
size_t moved = 0;
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_OK(aksl_fopen(path, "w", &fp));
AKSL_CHECK_OK(aksl_fwrite(payload, 1, sizeof(payload), fp));
AKSL_CHECK_OK(aksl_fwrite(payload, 1, sizeof(payload), fp, &moved));
AKSL_CHECK(moved == sizeof(payload));
AKSL_CHECK_OK(aksl_fclose(fp));
fp = NULL;
moved = 0;
memset(readback, 0x00, sizeof(readback));
AKSL_CHECK_OK(aksl_fopen(path, "r", &fp));
AKSL_CHECK_OK(aksl_fread(readback, 1, sizeof(readback), fp));
AKSL_CHECK_OK(aksl_fread(readback, 1, sizeof(readback), fp, &moved));
AKSL_CHECK(moved == sizeof(readback));
AKSL_CHECK_OK(aksl_fclose(fp));
AKSL_CHECK(memcmp(payload, readback, sizeof(payload)) == 0);
@@ -80,27 +119,35 @@ static int test_write_read_round_trip(void)
return 0;
}
/* Asking for more members than the file holds sets feof, which is AKERR_EOF. */
static int test_fread_short_read_is_eof(void)
/*
* Asking for more members than the file holds sets feof, which is AKERR_EOF --
* and nmemb_out says how many did arrive, which is the entire reason for
* distinguishing EOF from an error rather than reporting both as AKERR_IO.
*/
static int test_fread_short_read_is_eof_and_reports_the_count(void)
{
char path[AKSL_TMP_MAX];
char buf[32];
size_t moved = 0;
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_OK(aksl_fopen(path, "w", &fp));
AKSL_CHECK_OK(aksl_fwrite("abcd", 1, 4, fp));
AKSL_CHECK_OK(aksl_fwrite("abcd", 1, 4, fp, &moved));
AKSL_CHECK(moved == 4);
AKSL_CHECK_OK(aksl_fclose(fp));
fp = NULL;
moved = 99;
memset(buf, 0x00, sizeof(buf));
AKSL_CHECK_OK(aksl_fopen(path, "r", &fp));
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), fp),
AKERR_EOF, "EOF");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), fp, &moved),
AKERR_EOF, "EOF");
/* TODO.md 2.2.3: this is what the caller could not previously find out. */
AKSL_CHECK(moved == 4);
AKSL_CHECK_OK(aksl_fclose(fp));
/* The bytes that did arrive are still in the buffer. */
AKSL_CHECK(memcmp(buf, "abcd", 4) == 0);
AKSL_CHECK(unlink(path) == 0);
return 0;
@@ -108,59 +155,151 @@ static int test_fread_short_read_is_eof(void)
/*
* A stream opened "w" has no read permission, so fread sets the error indicator
* rather than the EOF one: AKERR_IO, not AKERR_EOF.
* rather than the EOF one -- and the status is the errno the C library actually
* saw, EBADF, rather than a flat AKERR_IO.
*
* That is a change from the old wrapper, which read ferror() and reported
* AKERR_IO without ever consulting errno. Routing it through AKSL_ERRNO_OR
* (TODO.md 2.2.1) keeps AKERR_IO as the fallback for the case where the stream
* is in error and errno says nothing, and hands back the real reason otherwise.
*/
static int test_fread_from_write_only_stream_is_io_error(void)
static int test_fread_from_write_only_stream_reports_errno(void)
{
char path[AKSL_TMP_MAX];
char buf[4];
size_t moved = 99;
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_OK(aksl_fopen(path, "w", &fp));
memset(buf, 0x00, sizeof(buf));
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), fp),
AKERR_IO, "Error reading file");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), fp, &moved),
EBADF, "of 4 members");
AKSL_CHECK(moved == 0);
AKSL_CHECK_OK(aksl_fclose(fp));
AKSL_CHECK(unlink(path) == 0);
return 0;
}
static int test_fread_rejects_null_stream(void)
static int test_fread_rejects_null_arguments(void)
{
char path[AKSL_TMP_MAX];
char buf[4];
size_t moved = 0;
FILE *fp = NULL;
memset(buf, 0x00, sizeof(buf));
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), NULL),
AKERR_NULLPOINTER, "NULL");
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_OK(aksl_fopen(path, "r", &fp));
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), NULL, &moved),
AKERR_NULLPOINTER, "fp=");
/* TODO.md 2.2.3: ptr was never checked in either direction. */
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(NULL, 1, sizeof(buf), fp, &moved),
AKERR_NULLPOINTER, "ptr=");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fread(buf, 1, sizeof(buf), fp, NULL),
AKERR_NULLPOINTER, "nmemb_out=");
AKSL_CHECK_OK(aksl_fclose(fp));
AKSL_CHECK(unlink(path) == 0);
return 0;
}
/* Mirror image of the fread case: a "r" stream cannot be written to. */
static int test_fwrite_to_read_only_stream_is_io_error(void)
static int test_fwrite_to_read_only_stream_reports_errno(void)
{
char path[AKSL_TMP_MAX];
size_t moved = 99;
FILE *fp = NULL;
AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0);
AKSL_CHECK_OK(aksl_fopen(path, "r", &fp));
AKSL_CHECK_STATUS(aksl_fwrite("xy", 1, 2, fp), AKERR_IO);
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fwrite("xy", 1, 2, fp, &moved),
EBADF, "wrote 0 of 2 members");
AKSL_CHECK(moved == 0);
AKSL_CHECK_OK(aksl_fclose(fp));
AKSL_CHECK(unlink(path) == 0);
return 0;
}
static int test_fwrite_rejects_null_stream(void)
static int test_fwrite_rejects_null_arguments(void)
{
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fwrite("xy", 1, 2, NULL),
AKERR_NULLPOINTER, "NULL");
size_t moved = 0;
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fwrite("xy", 1, 2, NULL, &moved),
AKERR_NULLPOINTER, "fp=");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fwrite(NULL, 1, 2, stdout, &moved),
AKERR_NULLPOINTER, "ptr=");
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fwrite("xy", 1, 2, stdout, NULL),
AKERR_NULLPOINTER, "nmemb_out=");
return 0;
}
/*
* /dev/full accepts an open for writing and then fails every write with ENOSPC.
* It is the only convenient way to reach a genuinely short write on a stream
* that is otherwise perfectly healthy.
*/
static int test_fwrite_to_full_device_reports_enospc(void)
{
char payload[8192];
size_t moved = 99;
FILE *fp = NULL;
if ( access("/dev/full", W_OK) != 0 ) {
fprintf(stderr, " (skipped: no writable /dev/full on this platform)\n");
return 0;
}
memset(payload, 'x', sizeof(payload));
AKSL_CHECK_OK(aksl_fopen("/dev/full", "w", &fp));
/*
* Larger than any plausible stdio buffer, so the write reaches the device
* inside fwrite rather than being deferred to fclose -- the deferred case is
* the next test.
*/
AKSL_CHECK_STATUS(aksl_fwrite(payload, 1, sizeof(payload), fp, &moved), ENOSPC);
AKSL_CHECK(moved < sizeof(payload));
/*
* fclose succeeds here, and that is not a contradiction: the write already
* reached the device and failed, so by close time there is nothing left
* buffered to fail on. The deferred case -- where the write fits in the
* stdio buffer and only fclose finds out -- is the next test. Take the
* context however it comes back rather than asserting either way, since
* whether the error indicator survives to fclose is a stdio implementation
* detail and not part of this library's contract.
*/
(void)aksl_take(aksl_fclose(fp));
return 0;
}
/*
* The write that fits in the stdio buffer succeeds, and the failure surfaces
* only when fclose flushes it. A caller who checks fwrite and ignores fclose
* loses the data silently, which is why aksl_fclose reports errno rather than
* just a non-zero return.
*/
static int test_fclose_surfaces_a_failed_flush(void)
{
size_t moved = 0;
FILE *fp = NULL;
if ( access("/dev/full", W_OK) != 0 ) {
fprintf(stderr, " (skipped: no writable /dev/full on this platform)\n");
return 0;
}
AKSL_CHECK_OK(aksl_fopen("/dev/full", "w", &fp));
AKSL_CHECK_OK(aksl_fwrite("small", 1, 5, fp, &moved));
AKSL_CHECK(moved == 5);
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fclose(fp), ENOSPC, "buffered data is lost");
return 0;
}
static int test_fclose_rejects_null_stream(void)
{
AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fclose(NULL),
AKERR_NULLPOINTER, "NULL");
AKERR_NULLPOINTER, "NULL");
return 0;
}
@@ -172,16 +311,19 @@ int main(void)
AKSL_RUN(failures, test_fopen_writes_stream_pointer);
AKSL_RUN(failures, test_fopen_reports_missing_path);
AKSL_RUN(failures, test_fopen_rejects_null_stream_out);
AKSL_RUN(failures, test_fopen_reports_permission_denied);
AKSL_RUN(failures, test_fopen_rejects_null_arguments);
AKSL_RUN(failures, test_write_read_round_trip);
AKSL_RUN(failures, test_fread_short_read_is_eof);
AKSL_RUN(failures, test_fread_from_write_only_stream_is_io_error);
AKSL_RUN(failures, test_fread_rejects_null_stream);
AKSL_RUN(failures, test_fread_short_read_is_eof_and_reports_the_count);
AKSL_RUN(failures, test_fread_from_write_only_stream_reports_errno);
AKSL_RUN(failures, test_fread_rejects_null_arguments);
AKSL_RUN(failures, test_fwrite_to_read_only_stream_is_io_error);
AKSL_RUN(failures, test_fwrite_rejects_null_stream);
AKSL_RUN(failures, test_fwrite_to_read_only_stream_reports_errno);
AKSL_RUN(failures, test_fwrite_rejects_null_arguments);
AKSL_RUN(failures, test_fwrite_to_full_device_reports_enospc);
AKSL_RUN(failures, test_fclose_surfaces_a_failed_flush);
AKSL_RUN(failures, test_fclose_rejects_null_stream);
AKSL_REPORT(failures);