diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..11723fd --- /dev/null +++ b/.gitignore @@ -0,0 +1,27 @@ +# Build trees. The names are conventional rather than enforced -- AGENTS.md uses +# build/, build-asan/ and build-coverage/ -- so anything build* is ignored. +build*/ + +# Profiling output. -pg is no longer in the default build (see CMakeLists.txt), +# but a deliberate profiling run still drops these wherever it was started. +gmon.out +*.gcda +*.gcno +*.gcov + +# Test and report artifacts written into the source root by CI-style invocations. +ctest-junit.xml +mutation-junit.xml +coverage-summary.txt +coverage.xml + +# Emacs. The backup files and the lock symlinks both; a dangling .#file symlink +# pointing at user@host:pid is not something anyone wants in a diff. +*~ +\#*\# +.\#* + +# Editor and OS noise +*.swp +.DS_Store +compile_commands.json diff --git a/CMakeLists.txt b/CMakeLists.txt index e9d4b1c..4ddce08 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,9 +28,42 @@ configure_file( @ONLY ) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -ggdb -pg") -set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -g -ggdb -pg") -set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -g -ggdb -pg") +# This used to be three lines setting CMAKE_CXX_FLAGS and the linker flags to +# "-g -ggdb -pg". In a LANGUAGES C project the CXX variable reaches no compiler +# at all, so the debug flags never applied -- but -pg did reach the linkers, and +# every test binary that ran dropped a gmon.out in the working directory. Nobody +# was reading those profiles. CMAKE_BUILD_TYPE controls debug info now, which is +# what it is for; add -pg deliberately when you actually want to profile. +# +# Debug is the default for a bare `cmake -S . -B build` because this is a +# library under active development and a stripped -O3 build is not what anyone +# configuring it without an opinion is asking for. +if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) + set(CMAKE_BUILD_TYPE Debug CACHE STRING "Build type" FORCE) + message(STATUS "CMAKE_BUILD_TYPE was unset; defaulting to Debug") +endif() + +# Warnings. The only warning in the tree when these went on was the unused +# `queue` parameter of aksl_tree_iterate, which TODO.md 2.2.8 had already +# recorded as a dead parameter -- so the cost of turning them on was one +# already-known defect, and the cost of leaving them off was every future one. +# +# -Wpedantic is deliberately not here. libakerror's FAIL_* macros take a message +# plus varargs, and a call with a bare message and no arguments trips "ISO C99 +# requires at least one argument for the ...", which is a complaint about the +# macro's shape rather than about anything at this call site. Every FAIL_* in +# src/stdlib.c passes at least one argument regardless -- see the note in +# TODO.md 2.3 -- so the code is pedantic-clean; it is the expansion that is not. +if(CMAKE_C_COMPILER_ID MATCHES "^(GNU|Clang|AppleClang)$") + add_compile_options(-Wall -Wextra) + # CI turns this on. Locally it is off, because a warning that stops the build + # while you are in the middle of something is a good way to teach people to + # turn warnings off. + option(AKSL_WERROR "Treat compiler warnings as errors" OFF) + if(AKSL_WERROR) + add_compile_options(-Werror) + endif() +endif() # Sanitizer build, off by default: # cmake -S . -B build-asan -DAKSL_SANITIZE=ON && ctest --test-dir build-asan @@ -263,6 +296,7 @@ set(AKSL_TESTS status_registry stream strhash + strto tree version ) @@ -270,11 +304,13 @@ set(AKSL_TESTS set(AKSL_WILL_FAIL_TESTS ) +# Empty, and that is the news. It held four entries -- convert_strict, +# list_append_chain, list_iterate_head and tree_iterate_break -- one for each +# confirmed defect in TODO.md 2.1. All four are fixed, so each of those files +# was folded back into the test for the thing it was testing (tests/ +# test_convert.c, tests/test_linkedlist.c and tests/test_tree.c) where it now +# has to keep passing rather than merely keep failing visibly. set(AKSL_KNOWN_FAILING_TESTS - convert_strict # TODO.md 2.1.5 -- ato* cannot report a bad conversion - list_append_chain # TODO.md 2.1.1 -- append truncates lists of 2+ nodes - list_iterate_head # TODO.md 2.1.2 -- iterate starts at the midpoint - tree_iterate_break # TODO.md 2.1.3 -- ITERATOR_BREAK does not stop a walk ) foreach(_test IN LISTS AKSL_TESTS AKSL_WILL_FAIL_TESTS AKSL_KNOWN_FAILING_TESTS) @@ -291,6 +327,18 @@ foreach(_test IN LISTS AKSL_TESTS AKSL_WILL_FAIL_TESTS AKSL_KNOWN_FAILING_TESTS) list(APPEND AKSL_TEST_TARGETS test_${_test}) endforeach() +# tests/test_memory.c asks for SIZE_MAX/2 bytes to check that a refused +# allocation reports ENOMEM and leaves *dst NULL. Plain malloc returns NULL and +# sets ENOMEM, which is the case under test; ASan's allocator instead treats a +# request that large as a bug in the caller and aborts the process before malloc +# ever returns. allocator_may_return_null=1 restores the libc behaviour for this +# one binary, so the sanitizer build tests the same contract as the normal one +# rather than skipping it. +if(AKSL_SANITIZE AND "memory" IN_LIST AKSL_TESTS) + set_tests_properties(memory PROPERTIES + ENVIRONMENT "ASAN_OPTIONS=allocator_may_return_null=1") +endif() + if(AKSL_WILL_FAIL_TESTS OR AKSL_KNOWN_FAILING_TESTS) set_tests_properties( ${AKSL_WILL_FAIL_TESTS} ${AKSL_KNOWN_FAILING_TESTS} diff --git a/include/akstdlib.h b/include/akstdlib.h index 6eee423..6c579aa 100644 --- a/include/akstdlib.h +++ b/include/akstdlib.h @@ -29,10 +29,37 @@ */ #include -#include -#include -#include +/* + * What this header needs in its own declarations, and no more: + * stdio.h FILE + * stddef.h size_t + * stdint.h uint32_t + * stdarg.h va_list, for the v* forms of the formatted-output wrappers + * + * It used to pull in stdlib.h and string.h as well, which nothing here needs and + * which every consumer then got whether it wanted them or not. stddef.h in place + * of stdlib.h is the same size_t at a fraction of the namespace. TODO.md 2.2.16. + */ +#include +#include #include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/* + * Restore the compile-time format/argument checking that callers otherwise lose + * by going through a variadic wrapper: without it, printf("%d", "str") is caught + * and aksl_printf(&n, "%d", "str") is not. TODO.md 2.2.5. + */ +#if defined(__GNUC__) || defined(__clang__) +#define AKSL_PRINTF_FORMAT(__fmt_index, __first_arg) \ + __attribute__((format(printf, __fmt_index, __first_arg))) +#else +#define AKSL_PRINTF_FORMAT(__fmt_index, __first_arg) +#endif typedef struct aksl_ListNode { void *data; @@ -47,13 +74,27 @@ typedef struct aksl_TreeNode { void *leaf; } aksl_TreeNode; -#define AKSL_TREE_SEARCH_BFS 0 /** Breadth-first search mode for tree nodes. Currently unsupported. */ -#define AKSL_TREE_SEARCH_BFS_RIGHT 1 /** Right-hand breadth-first search mode for tree nodes. Currentl unsupported. */ +#define AKSL_TREE_SEARCH_BFS 0 /** Breadth-first (left child before right) search mode for tree nodes */ +#define AKSL_TREE_SEARCH_BFS_RIGHT 1 /** Breadth-first, right child before left, search mode for tree nodes */ #define AKSL_TREE_SEARCH_DFS 2 /** Alias for AKSL_TREE_SEARCH_DFS_PREORDER */ #define AKSL_TREE_SEARCH_DFS_PREORDER 2 /** Depth first pre-order (root, left, right) search mode for tree nodes */ -#define AKSL_TREE_SEARCH_DFS_INORDER 3 /** Depth first in-order (left, root, right) search mode for tree nodes. Currently unsupported. */ -#define AKSL_TREE_SEARCH_DFS_POSTORDER 4 /** Depth first post-order (left, right, root) search mode for tree nodes. Currently unsupported. */ -#define AKSL_TREE_SEARCH_VISIT 5 /** Used when iterating through a tree structure as a control flag: don't traverse the children, just visit the node */ +#define AKSL_TREE_SEARCH_DFS_INORDER 3 /** Depth first in-order (left, root, right) search mode for tree nodes */ +#define AKSL_TREE_SEARCH_DFS_POSTORDER 4 /** Depth first post-order (left, right, root) search mode for tree nodes */ +#define AKSL_TREE_SEARCH_VISIT 5 /** Visit the node and stop; do not traverse the children */ + +/* + * How deep aksl_tree_iterate will walk before raising AKERR_OUTOFBOUNDS. + * + * This is a real limit, not a formality: the depth-first walk is recursive, so + * without it a degenerate chain overflows the stack and a child pointing back at + * an ancestor recurses until the process dies. 256 is far past any balanced tree + * that fits in memory (2^256 nodes) and comfortably short of a stack overflow, + * so in practice it only ever rejects the degenerate shapes it exists to reject. + * + * It also sizes the ancestor buffer aksl_tree_iterate keeps on its own stack -- + * one pointer per level, so raising it costs 8 bytes of stack per level. + */ +#define AKSL_TREE_MAX_DEPTH 256 typedef akerr_ErrorContext AKERR_NOIGNORE *(*aksl_ListNodeIterator)(aksl_ListNode *node, void *data); typedef akerr_ErrorContext AKERR_NOIGNORE *(*aksl_TreeNodeIterator)(aksl_TreeNode *node, void *data); @@ -96,35 +137,118 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_version_check(int major, int minor, int #define AKSL_VERSION_CHECK() \ aksl_version_check(AKSL_VERSION_MAJOR, AKSL_VERSION_MINOR, AKSL_VERSION_PATCH) -akerr_ErrorContext AKERR_NOIGNORE *aksl_fopen(char *pathname, char *mode, FILE **fp); -akerr_ErrorContext AKERR_NOIGNORE *aksl_fread(void *ptr, size_t size, size_t nmemb, FILE *stream); -akerr_ErrorContext AKERR_NOIGNORE *aksl_fwrite(void *ptr, size_t size, size_t nmemb, FILE *fp); +/* + * Stream I/O. + * + * fread and fwrite take a required size_t *nmemb_out. It is always written -- + * including on the failure paths -- so a caller who gets AKERR_EOF can find out + * how much data arrived before the stream ran out, which is the whole point of + * distinguishing EOF from an error. A transfer that comes up short with no + * stream error set is AKERR_IO, not silent success. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_fopen(const char *pathname, const char *mode, FILE **fp); +akerr_ErrorContext AKERR_NOIGNORE *aksl_fread(void *ptr, size_t size, size_t nmemb, FILE *stream, size_t *nmemb_out); +akerr_ErrorContext AKERR_NOIGNORE *aksl_fwrite(const void *ptr, size_t size, size_t nmemb, FILE *fp, size_t *nmemb_out); akerr_ErrorContext AKERR_NOIGNORE *aksl_fclose(FILE *stream); +/* + * Memory. aksl_malloc and aksl_calloc reject a zero-size request outright rather + * than reporting whatever errno happened to hold when malloc(0) returned NULL. + * aksl_realloc takes the pointer by reference and leaves it valid and untouched + * on failure, which is realloc(3)'s classic leak closed off. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_malloc(size_t size, void **dst); -akerr_ErrorContext AKERR_NOIGNORE *aksl_memset(void *s, int c, size_t n); -akerr_ErrorContext AKERR_NOIGNORE *aksl_memcpy(void *d, void *s, size_t n); +akerr_ErrorContext AKERR_NOIGNORE *aksl_calloc(size_t nmemb, size_t size, void **dst); +akerr_ErrorContext AKERR_NOIGNORE *aksl_realloc(void **ptr, size_t size); akerr_ErrorContext AKERR_NOIGNORE *aksl_free(void *ptr); +akerr_ErrorContext AKERR_NOIGNORE *aksl_freep(void **ptr); +akerr_ErrorContext AKERR_NOIGNORE *aksl_memset(void *s, int c, size_t n); +/* Overlapping ranges are AKERR_VALUE, not undefined behaviour; use aksl_memmove. */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_memcpy(void *d, const void *s, size_t n); +akerr_ErrorContext AKERR_NOIGNORE *aksl_memmove(void *d, const void *s, size_t n); +akerr_ErrorContext AKERR_NOIGNORE *aksl_memcmp(const void *a, const void *b, size_t n, int *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_memchr(const void *s, int c, size_t n, void **dest); -akerr_ErrorContext AKERR_NOIGNORE *aksl_printf(int *count, const char *restrict format, ...); -akerr_ErrorContext AKERR_NOIGNORE *aksl_fprintf(int *count, FILE *restrict stream, const char *restrict format, ...); -akerr_ErrorContext AKERR_NOIGNORE *aksl_sprintf(int *count, char *restrict str, const char *restrict format, ...); +/* + * Formatted output. *count is the byte count written excluding the terminating + * NUL, and is 0 on every failure path. + * + * There is no aksl_sprintf. It wrapped vsprintf, which cannot be bounded, and an + * error-handling wrapper around an unbounded write is the sharp edge this + * library exists to remove. aksl_snprintf replaces it and treats truncation as + * AKERR_OUTOFBOUNDS rather than as a short success. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_printf(int *count, const char *restrict format, ...) AKSL_PRINTF_FORMAT(2, 3); +akerr_ErrorContext AKERR_NOIGNORE *aksl_fprintf(int *count, FILE *restrict stream, const char *restrict format, ...) AKSL_PRINTF_FORMAT(3, 4); +akerr_ErrorContext AKERR_NOIGNORE *aksl_snprintf(int *count, char *restrict str, size_t size, const char *restrict format, ...) AKSL_PRINTF_FORMAT(4, 5); + +akerr_ErrorContext AKERR_NOIGNORE *aksl_vprintf(int *count, const char *restrict format, va_list args); +akerr_ErrorContext AKERR_NOIGNORE *aksl_vfprintf(int *count, FILE *restrict stream, const char *restrict format, va_list args); +akerr_ErrorContext AKERR_NOIGNORE *aksl_vsnprintf(int *count, char *restrict str, size_t size, const char *restrict format, va_list args); + +/* + * String to number. + * + * These are strict, unlike the libc functions they are named for: + * + * no digits consumed -> AKERR_VALUE + * trailing junk, endptr NULL -> AKERR_VALUE + * value outside the type -> ERANGE + * + * and *dest is 0 on every failure. Pass a non-NULL endptr to opt out of the + * trailing-junk check and read a number off the front of a longer string. + * + * The ato* forms are aksl_strto* with base 10 and a NULL endptr, so the whole + * string must be a number. "0x10" is AKERR_VALUE through aksl_atoi -- base 10 + * stops at the 'x' -- and 16 through aksl_strtol(nptr, NULL, 0, &dest). + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtol(const char *nptr, char **endptr, int base, long *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoll(const char *nptr, char **endptr, int base, long long *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoul(const char *nptr, char **endptr, int base, unsigned long *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoull(const char *nptr, char **endptr, int base, unsigned long long *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtod(const char *nptr, char **endptr, double *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtof(const char *nptr, char **endptr, float *dest); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtold(const char *nptr, char **endptr, long double *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_atoi(const char *nptr, int *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_atol(const char *nptr, long *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_atoll(const char *nptr, long long *dest); akerr_ErrorContext AKERR_NOIGNORE *aksl_atof(const char *nptr, double *dest); -akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath(const char *restrict path, char *restrict resolved_path); +/* + * realpath(3). `buflen` must be at least PATH_MAX, because realpath(3) offers no + * way to bound its own write -- an undersized buffer is AKERR_OUTOFBOUNDS rather + * than a buffer overflow. aksl_realpath_alloc sizes and allocates the result + * instead; *dest is then the caller's to release with aksl_free. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath(const char *restrict path, char *restrict resolved_path, size_t buflen); +akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath_alloc(const char *restrict path, char **dest); -akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2(char *str, size_t len, uint32_t *hashval); +/* + * djb2. Length-driven, so embedded NUL bytes hash like any other byte; bytes + * >= 0x80 are read as unsigned, so the result matches canonical djb2 and does + * not depend on whether plain char is signed on the target. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2(const char *str, size_t len, uint32_t *hashval); +akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2_str(const char *str, uint32_t *hashval); // Linked list functions +akerr_ErrorContext AKERR_NOIGNORE *aksl_list_node_init(aksl_ListNode *node, void *data); akerr_ErrorContext AKERR_NOIGNORE *aksl_list_append(aksl_ListNode *list, aksl_ListNode *obj); -akerr_ErrorContext AKERR_NOIGNORE *aksl_list_pop(aksl_ListNode *node); +/* + * `head` is required. Popping the head node has to move the caller's own head + * pointer, and there is no way to do that from a node pointer alone -- the old + * one-argument form left the caller aimed at a detached node. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_list_pop(aksl_ListNode **head, aksl_ListNode *node); akerr_ErrorContext AKERR_NOIGNORE *aksl_list_iterate(aksl_ListNode *list, aksl_ListNodeIterator iter, void *data); // Tree functions -akerr_ErrorContext AKERR_NOIGNORE *aksl_tree_iterate(aksl_TreeNode *root, aksl_TreeNodeIterator iter, aksl_AllocFunc lalloc, aksl_FreeFunc lfree, uint8_t searchmode, void *data, aksl_ListNode *queue); +akerr_ErrorContext AKERR_NOIGNORE *aksl_tree_node_init(aksl_TreeNode *node, void *leaf); +akerr_ErrorContext AKERR_NOIGNORE *aksl_tree_iterate(aksl_TreeNode *root, aksl_TreeNodeIterator iter, aksl_AllocFunc lalloc, aksl_FreeFunc lfree, uint8_t searchmode, void *data); + +#ifdef __cplusplus +} +#endif #endif diff --git a/src/stdlib.c b/src/stdlib.c index d3727ec..15b0549 100644 --- a/src/stdlib.c +++ b/src/stdlib.c @@ -1,10 +1,26 @@ #include -#include -#include +#include #include -#include +#include #include #include +#include +#include +#include + +/* + * errno as an akerror status, with a fallback for when it is not one. + * + * A libc call is allowed to fail without touching errno -- malloc(0) may return + * NULL and leave it alone -- and errno may equally be a leftover from some + * earlier, unrelated, *successful* call. Reporting it raw produced a FAIL whose + * status was 0, which every downstream DETECT and CATCH reads as success while + * the context still holds a pool slot: an error that is invisible and leaks at + * the same time. Every errno-sourced status in this file goes through here, and + * every wrapped call clears errno first so the value read back is its own. + * TODO.md 2.2.1. + */ +#define AKSL_ERRNO_OR(__fallback) (errno != 0 ? errno : (__fallback)) /* * Version of the library itself. These read the AKSL_VERSION_* macros as they @@ -57,15 +73,68 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_version_check(int major, int minor, int SUCCEED_RETURN(e); } +/* + * malloc(0) is the awkward case, and it is why *dst is cleared first and errno + * is cleared before the call. The standard lets malloc(0) return either a unique + * pointer or NULL, and a NULL return there is not a failure and need not set + * errno -- so the old code raised an error whose status was whatever errno + * happened to be holding, quite possibly 0. A zero-size request is rejected on + * its own terms instead, since there is nothing useful to hand back either way. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_malloc(size_t size, void **dst) { PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, dst, AKERR_NULLPOINTER, "NULL"); + FAIL_ZERO_RETURN(e, dst, AKERR_NULLPOINTER, "dst=%p", (void *)dst); + *dst = NULL; + FAIL_ZERO_RETURN(e, size, AKERR_VALUE, "zero-size allocation"); + errno = 0; *dst = malloc(size); - FAIL_ZERO_RETURN(e, *dst, errno, "%ld bytes", size); + FAIL_ZERO_RETURN(e, *dst, AKSL_ERRNO_OR(ENOMEM), "%zu bytes", size); SUCCEED_RETURN(e); } +akerr_ErrorContext AKERR_NOIGNORE *aksl_calloc(size_t nmemb, size_t size, void **dst) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, dst, AKERR_NULLPOINTER, "dst=%p", (void *)dst); + *dst = NULL; + FAIL_ZERO_RETURN(e, nmemb, AKERR_VALUE, "zero-size allocation (nmemb=0)"); + FAIL_ZERO_RETURN(e, size, AKERR_VALUE, "zero-size allocation (size=0)"); + errno = 0; + *dst = calloc(nmemb, size); + FAIL_ZERO_RETURN(e, *dst, AKSL_ERRNO_OR(ENOMEM), "%zu x %zu bytes", nmemb, size); + SUCCEED_RETURN(e); +} + +/* + * realloc(3)'s trap, hidden: on failure it returns NULL *and leaves the original + * block valid*, so the near-universal `p = realloc(p, n)` leaks the original + * every time it fails. Here the old pointer goes in and out through the same + * out-param, and is left untouched -- still valid, still the caller's to free -- + * whenever an error is raised. TODO.md 3.1. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_realloc(void **ptr, size_t size) +{ + void *resized = NULL; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, ptr, AKERR_NULLPOINTER, "ptr=%p", (void *)ptr); + FAIL_ZERO_RETURN(e, size, AKERR_VALUE, + "zero-size realloc; use aksl_free to release the block"); + errno = 0; + resized = realloc(*ptr, size); + FAIL_ZERO_RETURN(e, resized, AKSL_ERRNO_OR(ENOMEM), + "%zu bytes; the original block at %p is still valid", size, *ptr); + *ptr = resized; + SUCCEED_RETURN(e); +} + +/* + * free(NULL) is legal C and does nothing. This treats it as an error anyway, + * deliberately and against libc: in a codebase that routes every allocation + * through aksl_malloc, freeing a pointer you believed was live and finding it + * NULL means something upstream did not happen, and silence there is how a + * double-free or a lost allocation gets found three weeks later. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_free(void *ptr) { PREPARE_ERROR(e); @@ -74,174 +143,676 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_free(void *ptr) SUCCEED_RETURN(e); } +/* + * Frees and clears in one step, so the pointer cannot be used or freed twice. + * TODO.md 2.2.13 -- aksl_free leaves the caller holding a dangling pointer, and + * "remember to NULL it afterwards" is exactly the discipline this library is + * supposed to make unnecessary rather than merely possible. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_freep(void **ptr) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, ptr, AKERR_NULLPOINTER, "ptr=%p", (void *)ptr); + FAIL_ZERO_RETURN(e, *ptr, AKERR_NULLPOINTER, "*ptr is NULL"); + free(*ptr); + *ptr = NULL; + SUCCEED_RETURN(e); +} + +/* + * memset(3) and memcpy(3) cannot fail. They return their destination pointer + * unconditionally, so the old FAIL_ZERO_RETURN(e, memset(...), ...) and + * (memcpy(...) == d) checks were dead code that read as though there were a + * failure mode to catch -- TODO.md 2.2.11. What is worth checking is the + * arguments, which is all that is checked now. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_memset(void *s, int c, size_t n) { PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, s, AKERR_NULLPOINTER, "s=%p", s); - FAIL_ZERO_RETURN(e, memset(s, c, n), errno, "Failed to memset"); + memset(s, c, n); SUCCEED_RETURN(e); } +/* + * Overlapping ranges are rejected rather than left as undefined behaviour. This + * is the one place this wrapper is stricter than memcpy(3) instead of merely + * louder: memcpy with overlap is UB that usually appears to work, right up until + * a compiler version or a buffer length changes and it does not. Callers who + * mean to overlap want aksl_memmove, which is what memmove(3) is for. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_memcpy(void *d, const void *s, size_t n) +{ + /* + * Through uintptr_t: comparing two pointers that are not into the same + * object is undefined behaviour in C, and "is one inside the other" is + * precisely a comparison of two pointers we do not yet know are related. + */ + uintptr_t dst = (uintptr_t)d; + uintptr_t src = (uintptr_t)s; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, d, AKERR_NULLPOINTER, "d=%p, s=%p", d, s); + FAIL_ZERO_RETURN(e, s, AKERR_NULLPOINTER, "d=%p, s=%p", d, s); + FAIL_NONZERO_RETURN(e, (n > 0 && dst < src + n && src < dst + n), AKERR_VALUE, + "d=%p and s=%p overlap over %zu bytes; use aksl_memmove", + d, s, n); + memcpy(d, s, n); + SUCCEED_RETURN(e); +} -akerr_ErrorContext AKERR_NOIGNORE *aksl_memcpy(void *d, void *s, size_t n) +akerr_ErrorContext AKERR_NOIGNORE *aksl_memmove(void *d, const void *s, size_t n) { PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, d, AKERR_NULLPOINTER, "d=%p, s=%p", d, s); FAIL_ZERO_RETURN(e, s, AKERR_NULLPOINTER, "d=%p, s=%p", d, s); - FAIL_ZERO_RETURN(e, (memcpy(d, s, n) == d), errno, "Failed to memcpy"); + memmove(d, s, n); SUCCEED_RETURN(e); } +/* + * memcmp and memchr report their answer through an out-param rather than the + * return value, because the return value is already spoken for by the error + * context. A memchr that finds nothing is not an error -- *dest is NULL and the + * call succeeds -- since "absent" is an ordinary answer to that question. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_memcmp(const void *a, const void *b, size_t n, int *dest) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, a, AKERR_NULLPOINTER, "a=%p, b=%p, dest=%p", a, b, (void *)dest); + FAIL_ZERO_RETURN(e, b, AKERR_NULLPOINTER, "a=%p, b=%p, dest=%p", a, b, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "a=%p, b=%p, dest=%p", a, b, (void *)dest); + *dest = memcmp(a, b, n); + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_memchr(const void *s, int c, size_t n, void **dest) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, s, AKERR_NULLPOINTER, "s=%p, dest=%p", s, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "s=%p, dest=%p", s, (void *)dest); + *dest = memchr(s, c, n); + SUCCEED_RETURN(e); +} + +/* + * pathname and mode are checked. fopen(NULL, ...) is undefined behaviour, and + * this wrapper used to hand both straight through unexamined -- reachable from + * user input in practice, which is why akbasic validates the filename itself + * before calling DLOAD/DSAVE with a comment pointing at TODO.md 2.2.2. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_fopen( - char *pathname, - char *mode, + const char *pathname, + const char *mode, FILE **fp) { PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "NULL"); + FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "pathname=%p, mode=%p, fp=%p", + (void *)pathname, (void *)mode, (void *)fp); + *fp = NULL; + FAIL_ZERO_RETURN(e, pathname, AKERR_NULLPOINTER, "pathname=%p, mode=%p, fp=%p", + (void *)pathname, (void *)mode, (void *)fp); + FAIL_ZERO_RETURN(e, mode, AKERR_NULLPOINTER, "pathname=%p, mode=%p, fp=%p", + (void *)pathname, (void *)mode, (void *)fp); + errno = 0; *fp = fopen(pathname, mode); - FAIL_ZERO_RETURN(e, *fp, errno, "%s", pathname); + FAIL_ZERO_RETURN(e, *fp, AKSL_ERRNO_OR(AKERR_IO), "%s", pathname); SUCCEED_RETURN(e); } +/* + * fread and fwrite both report how much they actually transferred, through a + * required out-param. Three things were wrong with the old pair (TODO.md 2.2.3), + * and the count is the fix for the worst of them: a caller who got AKERR_EOF had + * no way to find out how much data had arrived before the stream ran out, which + * makes the EOF status almost useless for the partial-read case it exists to + * report. *nmemb_out is always written, including on the error paths. + * + * The other two: ptr was never NULL-checked in either function, and a transfer + * that came up short with neither feof nor ferror set fell straight through to + * SUCCEED_RETURN -- a short transfer reported as complete success, which is the + * exact failure mode this library exists to prevent. That case is now AKERR_IO. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_fread( void *ptr, size_t size, size_t nmemb, - FILE *fp) + FILE *fp, + size_t *nmemb_out) { - size_t nmemr; + size_t nmemr = 0; PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "NULL"); + FAIL_ZERO_RETURN(e, nmemb_out, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + *nmemb_out = 0; + FAIL_ZERO_RETURN(e, ptr, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + errno = 0; nmemr = fread(ptr, size, nmemb, fp); + *nmemb_out = nmemr; if ( nmemr != nmemb ) { - FAIL_NONZERO_RETURN(e, feof(fp), AKERR_EOF, "EOF reached"); - FAIL_NONZERO_RETURN(e, ferror(fp), AKERR_IO, "Error reading file"); + FAIL_NONZERO_RETURN(e, ferror(fp), AKSL_ERRNO_OR(AKERR_IO), + "read %zu of %zu members", nmemr, nmemb); + FAIL_NONZERO_RETURN(e, feof(fp), AKERR_EOF, + "EOF after %zu of %zu members", nmemr, nmemb); + FAIL_RETURN(e, AKERR_IO, + "short read: %zu of %zu members, with neither EOF nor a stream error", + nmemr, nmemb); } SUCCEED_RETURN(e); } +/* + * ferror before feof, and no feof at all on the write path. The old version + * asked feof() first -- meaningless when writing -- and reported a failed write + * with the message "Error reading file". + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_fwrite( - void *ptr, + const void *ptr, size_t size, size_t nmemb, - FILE *fp) + FILE *fp, + size_t *nmemb_out) { - size_t nmemw; + size_t nmemw = 0; PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "NULL argument"); + FAIL_ZERO_RETURN(e, nmemb_out, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + *nmemb_out = 0; + FAIL_ZERO_RETURN(e, ptr, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + FAIL_ZERO_RETURN(e, fp, AKERR_NULLPOINTER, "ptr=%p, fp=%p, nmemb_out=%p", + ptr, (void *)fp, (void *)nmemb_out); + errno = 0; nmemw = fwrite(ptr, size, nmemb, fp); + *nmemb_out = nmemw; if ( nmemw != nmemb ) { - FAIL_NONZERO_RETURN(e, feof(fp), AKERR_EOF, "EOF reached"); - FAIL_NONZERO_RETURN(e, ferror(fp), AKERR_IO, "Error reading file"); + FAIL_NONZERO_RETURN(e, ferror(fp), AKSL_ERRNO_OR(AKERR_IO), + "wrote %zu of %zu members", nmemw, nmemb); + FAIL_RETURN(e, AKERR_IO, + "short write: %zu of %zu members, with no stream error set", + nmemw, nmemb); } SUCCEED_RETURN(e); } +/* + * Closing an already-closed stream is undefined behaviour and there is no way to + * detect it from here -- the FILE * is freed, so even reading it to check is the + * bug. This wrapper cannot help; do not do it. + */ akerr_ErrorContext AKERR_NOIGNORE *aksl_fclose(FILE *stream) { PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, stream, AKERR_NULLPOINTER, "NULL"); - FAIL_NONZERO_RETURN(e, fclose(stream), errno, "Failed to fclose"); + errno = 0; + FAIL_NONZERO_RETURN(e, fclose(stream), AKSL_ERRNO_OR(AKERR_IO), + "fclose failed, and any buffered data is lost"); + SUCCEED_RETURN(e); +} + +/* + * Formatted output. + * + * The va_list forms below do the work and the variadic forms are thin wrappers + * over them, which is both what ยง3.1 of TODO.md asked for -- so consumers can + * build their own variadic wrappers -- and what makes the va_end rule below + * checkable in one place instead of three. + * + * Three things every one of these now does that the old versions did not: + * + * - va_end matches every va_start, on the failure path as well as the happy + * one. Omitting it is undefined behaviour per the standard and leaks + * register-save state on some ABIs. akbasic's text sink ran this UB on every + * line of program output without anything visibly misbehaving, which is + * exactly what made it worth fixing before something did. TODO.md 2.1.4. + * - *count is written on every path. It used to be left holding vprintf's -1 + * after a failure, so a caller who read the length rather than the status got + * a negative byte count out of a function that had already failed. It is now + * 0 whenever an error is raised. + * - errno is cleared before the call and read back through AKSL_ERRNO_OR, so a + * failure can never be reported with a stale -- or with a zero -- status. + * + * aksl_sprintf is gone. It wrapped vsprintf, which cannot be bounded, and an + * error-handling wrapper around an unbounded write is precisely the sharp edge + * this library exists to remove. aksl_snprintf replaces it, and treats + * truncation as the failure it is rather than as a short success. TODO.md 2.2.4. + */ + +akerr_ErrorContext AKERR_NOIGNORE *aksl_vprintf(int *count, const char *restrict format, va_list args) +{ + int written = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, count, AKERR_NULLPOINTER, "count=%p, format=%p", (void *)count, (void *)format); + *count = 0; + FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, format=%p", (void *)count, (void *)format); + errno = 0; + written = vprintf(format, args); + FAIL_NONZERO_RETURN(e, (written < 0), AKSL_ERRNO_OR(AKERR_IO), "Short write"); + *count = written; SUCCEED_RETURN(e); } akerr_ErrorContext AKERR_NOIGNORE *aksl_printf(int *count, const char *restrict format, ...) { va_list args; - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, count, AKERR_NULLPOINTER, "count=%p, format=%p", (void *)count, (void *)format); - FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, format=%p", (void *)count, (void *)format); - va_start(args, format); - *count = vprintf(format, args); - FAIL_NONZERO_RETURN(e, (*count == -1), errno, "Short write"); - SUCCEED_RETURN(e); + akerr_ErrorContext *raised = NULL; + + va_start(args, format); + raised = aksl_vprintf(count, format, args); + va_end(args); + return raised; } +akerr_ErrorContext AKERR_NOIGNORE *aksl_vfprintf(int *count, FILE *restrict stream, const char *restrict format, va_list args) +{ + int written = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, count, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); + *count = 0; + FAIL_ZERO_RETURN(e, stream, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); + FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); + errno = 0; + written = vfprintf(stream, format, args); + FAIL_NONZERO_RETURN(e, (written < 0), AKSL_ERRNO_OR(AKERR_IO), "Short write"); + *count = written; + SUCCEED_RETURN(e); +} akerr_ErrorContext AKERR_NOIGNORE *aksl_fprintf(int *count, FILE *restrict stream, const char *restrict format, ...) { va_list args; - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, count, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); - FAIL_ZERO_RETURN(e, stream, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); - FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, stream=%p, format=%p", (void *)count, (void *)stream, (void *)format); + akerr_ErrorContext *raised = NULL; + va_start(args, format); - *count = vfprintf(stream, format, args); - FAIL_NONZERO_RETURN(e, (*count == -1), errno, "Short write"); - SUCCEED_RETURN(e); + raised = aksl_vfprintf(count, stream, format, args); + va_end(args); + return raised; } -akerr_ErrorContext AKERR_NOIGNORE *aksl_sprintf(int *count, char *restrict str, const char *restrict format, ...) +/* + * Truncation is an error, not a short success. snprintf(3) reports the length it + * *would* have written and silently drops the rest, which is the single most + * common way a bounded write goes wrong unnoticed; a caller who wanted to know + * would have had to compare the return against the buffer size by hand, which is + * the check this library exists to stop people forgetting. *count is the number + * of bytes written excluding the terminating NUL, and is 0 on any failure. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_vsnprintf(int *count, char *restrict str, size_t size, const char *restrict format, va_list args) { - va_list args; + int needed = 0; PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, count, AKERR_NULLPOINTER, "count=%p, str=%p, format=%p", (void *)count, (void *)str, (void *)format); + *count = 0; FAIL_ZERO_RETURN(e, str, AKERR_NULLPOINTER, "count=%p, str=%p, format=%p", (void *)count, (void *)str, (void *)format); FAIL_ZERO_RETURN(e, format, AKERR_NULLPOINTER, "count=%p, str=%p, format=%p", (void *)count, (void *)str, (void *)format); - va_start(args, format); - *count = vsprintf(str, format, args); - FAIL_NONZERO_RETURN(e, (*count == -1), errno, "Short write"); + FAIL_ZERO_RETURN(e, size, AKERR_VALUE, "size=0 leaves no room even for the terminating NUL"); + errno = 0; + 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; SUCCEED_RETURN(e); } -akerr_ErrorContext AKERR_NOIGNORE *aksl_atoi(const char *nptr, int *dest) +akerr_ErrorContext AKERR_NOIGNORE *aksl_snprintf(int *count, char *restrict str, size_t size, const char *restrict format, ...) { + va_list args; + akerr_ErrorContext *raised = NULL; + + va_start(args, format); + raised = aksl_vsnprintf(count, str, size, format, args); + va_end(args); + return raised; +} + +/* + * String to number. + * + * The ato* family used to be the one place in this library where a libc failure + * passed unnoticed: atoi("not a number") handed back success and 0, and + * atoi("99999999999999999999") handed back success and a wrapped value, because + * atoi(3) has no error channel at all. A library whose entire value proposition + * is turning silent libc failures into error contexts cannot ship that. + * TODO.md 2.1.5. + * + * The strto* wrappers below are the real implementation and the ato* wrappers + * are three-line calls into them, which is what TODO.md 3.1 asked for on its own + * account -- akbasic had to hand-write ~60 lines of exactly this (its + * src/convert.c) because the library would not do it. + * + * The contract, for every function here: + * + * no digits consumed -> AKERR_VALUE + * trailing junk, endptr NULL -> AKERR_VALUE + * value outside the type -> ERANGE + * *dest is 0 on every failure + * + * Pass a non-NULL endptr to opt out of the trailing-junk check and parse a + * number out of the front of a longer string; the ato* forms pass NULL, so they + * require the whole string to be a number. Leading whitespace is accepted + * throughout, as strtol(3) accepts it. + */ + +/* + * strtoul(3) and strtoull(3) accept a leading '-' and hand back the negation + * wrapped into the unsigned range, so "-1" parses as ULONG_MAX and reports no + * error whatsoever. That is a silent failure of precisely the kind this library + * exists to surface, so the sign is rejected before the call rather than after. + */ +static int has_negative_sign(const char *nptr) +{ + while ( isspace((unsigned char)*nptr) ) { + nptr++; + } + return (*nptr == '-') ? 1 : 0; +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtol(const char *nptr, char **endptr, int base, long *dest) +{ + char *end = NULL; + long value = 0; PREPARE_ERROR(e); FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - *dest = atoi(nptr); + *dest = 0; + errno = 0; + value = strtol(nptr, &end, base); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for long", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoll(const char *nptr, char **endptr, int base, long long *dest) +{ + char *end = NULL; + long long value = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0; + errno = 0; + value = strtoll(nptr, &end, base); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for long long", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoul(const char *nptr, char **endptr, int base, unsigned long *dest) +{ + char *end = NULL; + unsigned long value = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0; + FAIL_NONZERO_RETURN(e, has_negative_sign(nptr), AKERR_VALUE, + "\"%s\" is negative and the destination is unsigned", nptr); + errno = 0; + value = strtoul(nptr, &end, base); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for unsigned long", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtoull(const char *nptr, char **endptr, int base, unsigned long long *dest) +{ + char *end = NULL; + unsigned long long value = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0; + FAIL_NONZERO_RETURN(e, has_negative_sign(nptr), AKERR_VALUE, + "\"%s\" is negative and the destination is unsigned", nptr); + errno = 0; + value = strtoull(nptr, &end, base); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for unsigned long long", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +/* + * The floating-point forms report ERANGE for underflow as well as overflow, + * because that is the only signal strtod(3) gives and losing all the significant + * digits of "1e-400" is a conversion failure however it is spelled. "inf", + * "infinity" and "nan" are accepted, in any case, since strtod accepts them and + * they are exact round-trips rather than approximations of something else. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtod(const char *nptr, char **endptr, double *dest) +{ + char *end = NULL; + double value = 0.0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0.0; + errno = 0; + value = strtod(nptr, &end); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for double", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtof(const char *nptr, char **endptr, float *dest) +{ + char *end = NULL; + float value = 0.0f; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0.0f; + errno = 0; + value = strtof(nptr, &end); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for float", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_strtold(const char *nptr, char **endptr, long double *dest) +{ + char *end = NULL; + long double value = 0.0L; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0.0L; + errno = 0; + value = strtold(nptr, &end); + FAIL_NONZERO_RETURN(e, (end == nptr), AKERR_VALUE, "no digits in \"%s\"", nptr); + FAIL_NONZERO_RETURN(e, (errno == ERANGE), ERANGE, "\"%s\" is out of range for long double", nptr); + FAIL_NONZERO_RETURN(e, (errno != 0), errno, "\"%s\"", nptr); + if ( endptr != NULL ) { + *endptr = end; + } else { + FAIL_NONZERO_RETURN(e, (*end != '\0'), AKERR_VALUE, + "trailing junk \"%s\" after the number in \"%s\"", end, nptr); + } + *dest = value; + SUCCEED_RETURN(e); +} + +/* + * The ato* forms keep their libc-compatible names and signatures but not libc's + * silence: base 10, the whole string must be a number, and a value that does not + * fit the destination type is ERANGE. "0x10" is therefore AKERR_VALUE here -- + * base 10 stops at the 'x' and the rest is trailing junk. Reach for + * aksl_strtol(nptr, NULL, 0, &dest) if you want the 0x/0 prefixes honoured. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_atoi(const char *nptr, int *dest) +{ + long value = 0; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); + *dest = 0; + ATTEMPT { + CATCH(e, aksl_strtol(nptr, NULL, 10, &value)); + /* + * strtol already caught anything outside long. This narrows to int, and + * is compiled out where the two are the same width -- leaving it in + * would be a comparison that -Wtype-limits can prove is never true. + */ +#if LONG_MAX > INT_MAX + FAIL_NONZERO_BREAK(e, (value < INT_MIN || value > INT_MAX), ERANGE, + "\"%s\" is out of range for int", nptr); +#endif + *dest = (int)value; + } CLEANUP { + } PROCESS(e) { + } FINISH(e, true); SUCCEED_RETURN(e); } akerr_ErrorContext AKERR_NOIGNORE *aksl_atol(const char *nptr, long *dest) { - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - *dest = atol(nptr); - SUCCEED_RETURN(e); + return aksl_strtol(nptr, NULL, 10, dest); } akerr_ErrorContext AKERR_NOIGNORE *aksl_atoll(const char *nptr, long long *dest) { - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - *dest = atoll(nptr); - SUCCEED_RETURN(e); + return aksl_strtoll(nptr, NULL, 10, dest); } akerr_ErrorContext AKERR_NOIGNORE *aksl_atof(const char *nptr, double *dest) { - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, nptr, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "nptr=%p, dest=%p", (void *)nptr, (void *)dest); - *dest = atof(nptr); - SUCCEED_RETURN(e); + return aksl_strtod(nptr, NULL, dest); } -akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath(const char *restrict path, char *restrict resolved_path) +/* + * realpath(3), with the destination buffer made expressible. + * + * The old two-argument form had three separate problems, all of them reachable + * from ordinary use (TODO.md 2.1.6): resolved_path was never NULL-checked, so + * realpath(path, NULL) allocated a buffer the wrapper then discarded and leaked; + * there was no way for a caller to say how big the buffer was, so everyone had + * to know to supply PATH_MAX bytes; and the failure path formatted resolved_path + * with %s even though realpath(3) leaves its contents unspecified on failure -- + * so for the normal caller, who passed an uninitialised stack buffer, the error + * path itself read uninitialised memory and could crash. + * + * Taking the length fixes the first two and lets this refuse an undersized + * buffer up front, since realpath(3) offers no way to bound its own write. The + * message names the input path only. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath(const char *restrict path, char *restrict resolved_path, size_t buflen) { char *result = NULL; PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, path, AKERR_NULLPOINTER, "path=%p, dest=%p", (void *)path, (void *)resolved_path); + FAIL_ZERO_RETURN(e, path, AKERR_NULLPOINTER, "path=%p, resolved_path=%p", + (void *)path, (void *)resolved_path); + FAIL_ZERO_RETURN(e, resolved_path, AKERR_NULLPOINTER, "path=%p, resolved_path=%p", + (void *)path, (void *)resolved_path); + FAIL_NONZERO_RETURN(e, (buflen < PATH_MAX), AKERR_OUTOFBOUNDS, + "buflen %zu is below PATH_MAX (%d); realpath(3) cannot be bounded " + "below it, so a shorter buffer cannot be used safely", + buflen, PATH_MAX); + resolved_path[0] = '\0'; + errno = 0; result = realpath(path, resolved_path); - FAIL_ZERO_RETURN(e, result, errno, "path=%s, dest=%s", path, resolved_path); + FAIL_ZERO_RETURN(e, result, AKSL_ERRNO_OR(AKERR_IO), "path=%s", path); SUCCEED_RETURN(e); } -akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2(char *str, size_t len, uint32_t *hashval) +/* + * The allocating variant. realpath(path, NULL) sizes and allocates the buffer + * itself, which is the right answer when the caller has no PATH_MAX-sized buffer + * to hand; *dest is the caller's to release with aksl_free. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_realpath_alloc(const char *restrict path, char **dest) { + char *result = NULL; PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, str, AKERR_NULLPOINTER, "str"); - FAIL_ZERO_RETURN(e, hashval, AKERR_NULLPOINTER, "hashval"); + FAIL_ZERO_RETURN(e, path, AKERR_NULLPOINTER, "path=%p, dest=%p", (void *)path, (void *)dest); + FAIL_ZERO_RETURN(e, dest, AKERR_NULLPOINTER, "path=%p, dest=%p", (void *)path, (void *)dest); + *dest = NULL; + errno = 0; + result = realpath(path, NULL); + FAIL_ZERO_RETURN(e, result, AKSL_ERRNO_OR(AKERR_IO), "path=%s", path); + *dest = result; + SUCCEED_RETURN(e); +} + +/* + * djb2 over an explicit length, so embedded NUL bytes hash like any other byte. + * + * The cursor is an unsigned char *, not a char *. On x86 and ARM Linux plain + * char is signed, so every byte >= 0x80 used to contribute a sign-extended + * negative value: the hash differed from canonical djb2, and -- worse -- differed + * between platforms depending on the signedness of char. Benign for the 7-bit + * ASCII identifiers akbasic hashes, and quietly wrong for the first caller to + * key a table on a filename or a UTF-8 string literal. TODO.md 2.2.6. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2(const char *str, size_t len, uint32_t *hashval) +{ + const unsigned char *cursor = NULL; uint32_t h = 5381; + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, str, AKERR_NULLPOINTER, "str=%p, hashval=%p", (void *)str, (void *)hashval); + FAIL_ZERO_RETURN(e, hashval, AKERR_NULLPOINTER, "str=%p, hashval=%p", (void *)str, (void *)hashval); + cursor = (const unsigned char *)str; while (len--) { - h = ((h << 5) + h) + *str++; + h = ((h << 5) + h) + *cursor++; } *hashval = h; SUCCEED_RETURN(e); } +/* The NUL-terminated convenience form TODO.md 3.6 asked for. */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_strhash_djb2_str(const char *str, uint32_t *hashval) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, str, AKERR_NULLPOINTER, "str=%p, hashval=%p", (void *)str, (void *)hashval); + return aksl_strhash_djb2(str, strlen(str), hashval); +} + akerr_ErrorContext AKERR_NOIGNORE *aksl_list_append(aksl_ListNode *list, aksl_ListNode *obj) { PREPARE_ERROR(e); @@ -250,16 +821,32 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_list_append(aksl_ListNode *list, aksl_Li aksl_ListNode *slow = list; aksl_ListNode *fast = list; aksl_ListNode *tail = list; + /* + * Two separate walks, deliberately. The Floyd pass answers "is this list + * finite"; it says nothing about where the tail is, because `slow` stops at + * the midpoint. Conflating the two is what made this function truncate every + * list of two or more nodes -- see TODO.md 2.1.1. + */ while ( fast != NULL && fast->next != NULL ) { - tail = slow; slow = slow->next; fast = fast->next->next; if ( fast == slow) { - FAIL_RETURN(e, AKERR_CIRCULAR_REFERENCE, "%p", list); + FAIL_RETURN(e, AKERR_CIRCULAR_REFERENCE, "%p", (void *)list); } } - if ( fast != NULL ) { - tail = fast; + /* + * Only now is the list known to terminate, so this walk is guaranteed to. + * It doubles as the aliasing check: appending a node that is already in the + * list would relink it behind itself and orphan everything between its old + * position and the tail, so it is refused. The walk was happening anyway, + * which is what makes the check free. + */ + FAIL_NONZERO_RETURN(e, (tail == obj), AKERR_VALUE, + "obj %p is already the head of this list", (void *)obj); + while ( tail->next != NULL ) { + tail = tail->next; + FAIL_NONZERO_RETURN(e, (tail == obj), AKERR_VALUE, + "obj %p is already in this list", (void *)obj); } tail->next = obj; obj->next = NULL; @@ -267,91 +854,354 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_list_append(aksl_ListNode *list, aksl_Li SUCCEED_RETURN(e); } -akerr_ErrorContext AKERR_NOIGNORE *aksl_list_pop(aksl_ListNode *node) +/* + * `head` is required, not optional. Popping the head node used to leave the + * caller's own head pointer aimed at a node that is no longer in the list, and + * there was no way for the caller to learn the new one -- TODO.md 2.2.12. Taking + * the head by reference makes the correct call the only call that compiles. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_list_pop(aksl_ListNode **head, aksl_ListNode *node) { PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); + FAIL_ZERO_RETURN(e, head, AKERR_NULLPOINTER, "head=%p, node=%p", (void *)head, (void *)node); + FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "head=%p, node=%p", (void *)head, (void *)node); if ( node->prev != NULL ) { node->prev->next = node->next; } - if ( node->next != NULL ) { + if ( node->next != NULL ) { node->next->prev = node->prev; } + if ( *head == node ) { + *head = node->next; + } node->next = NULL; node->prev = NULL; SUCCEED_RETURN(e); } +/* + * Zeroing initialisers. Every caller previously had to remember to memset a node + * before its first use, because append and iterate both read next/prev -- and a + * stack-allocated node that skipped it walked into garbage. TODO.md 2.2.14. + */ +akerr_ErrorContext AKERR_NOIGNORE *aksl_list_node_init(aksl_ListNode *node, void *data) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); + node->data = data; + node->next = NULL; + node->prev = NULL; + SUCCEED_RETURN(e); +} + +akerr_ErrorContext AKERR_NOIGNORE *aksl_tree_node_init(aksl_TreeNode *node, void *leaf) +{ + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); + node->parent = NULL; + node->left = NULL; + node->right = NULL; + node->leaf = leaf; + SUCCEED_RETURN(e); +} + +/* + * One entry in the breadth-first traversal queue. Internal, and deliberately not + * aksl_ListNode: the walk needs to carry each node's depth alongside it, which is + * how a cyclic tree is stopped without a visited-set. The caller's lalloc/lfree + * allocate and release these -- which is what they were always documented to do + * and never actually did (TODO.md 2.2.8). + */ +typedef struct TreeQueueEntry { + struct TreeQueueEntry *next; + aksl_TreeNode *node; + int depth; +} TreeQueueEntry; + +/* Append one tree node to the tail of the traversal queue. */ +static akerr_ErrorContext AKERR_NOIGNORE *tree_queue_push( + TreeQueueEntry **head, + TreeQueueEntry **tail, + aksl_AllocFunc lalloc, + aksl_TreeNode *node, + int depth) +{ + TreeQueueEntry *entry = NULL; + PREPARE_ERROR(e); + ATTEMPT { + CATCH(e, lalloc(sizeof(TreeQueueEntry), (void **)&entry)); + FAIL_ZERO_BREAK(e, entry, AKERR_NULLPOINTER, + "allocator returned success but no memory"); + entry->next = NULL; + entry->node = node; + entry->depth = depth; + if ( *tail == NULL ) { + *head = entry; + } else { + (*tail)->next = entry; + } + *tail = entry; + } CLEANUP { + } PROCESS(e) { + } FINISH(e, true); + SUCCEED_RETURN(e); +} + +/* + * Release every entry still queued. Called from the CLEANUP block of the walk, + * so it runs on the error and break paths as well as the successful one. An + * lfree that itself fails cannot be allowed to abandon the rest of the queue, so + * the first such error is remembered and returned only once the queue is empty. + */ +static akerr_ErrorContext AKERR_NOIGNORE *tree_queue_drain( + TreeQueueEntry **head, + TreeQueueEntry **tail, + aksl_FreeFunc lfree) +{ + TreeQueueEntry *entry = NULL; + akerr_ErrorContext *first = NULL; + akerr_ErrorContext *this = NULL; + PREPARE_ERROR(e); + while ( *head != NULL ) { + entry = *head; + *head = entry->next; + this = lfree(entry); + if ( this != NULL && first == NULL ) { + first = this; + } else if ( this != NULL ) { + this = akerr_release_error(this); + } + } + *tail = NULL; + if ( first != NULL ) { + return first; + } + SUCCEED_RETURN(e); +} + +/* + * Breadth-first walk. Consumes the queue it is given; on any error it returns + * with the remainder still queued, and the caller's CLEANUP drains it. + * + * AKSL_TREE_SEARCH_BFS enqueues left before right, AKSL_TREE_SEARCH_BFS_RIGHT + * the other way round. A tree whose child points back at an ancestor grows the + * recorded depth without bound rather than looping forever, so it is caught by + * the depth cap -- as AKERR_OUTOFBOUNDS, not AKERR_CIRCULAR_REFERENCE, because + * unlike the depth-first walk this one has no ancestor chain to compare against. + */ +static akerr_ErrorContext AKERR_NOIGNORE *tree_bfs_walk( + TreeQueueEntry **head, + TreeQueueEntry **tail, + aksl_TreeNodeIterator iter, + aksl_AllocFunc lalloc, + aksl_FreeFunc lfree, + uint8_t searchmode, + void *data) +{ + TreeQueueEntry *entry = NULL; + aksl_TreeNode *node = NULL; + aksl_TreeNode *first = NULL; + aksl_TreeNode *second = NULL; + int depth = 0; + PREPARE_ERROR(e); + while ( *head != NULL ) { + entry = *head; + *head = entry->next; + if ( *head == NULL ) { + *tail = NULL; + } + node = entry->node; + depth = entry->depth; + /* + * Release the entry before doing anything that can fail, so the queue + * the caller's CLEANUP drains never contains an entry we already + * detached from it. + */ + PASS(e, lfree(entry)); + FAIL_NONZERO_RETURN(e, (depth >= AKSL_TREE_MAX_DEPTH), AKERR_OUTOFBOUNDS, + "tree deeper than AKSL_TREE_MAX_DEPTH (%d) at node %p", + AKSL_TREE_MAX_DEPTH, (void *)node); + PASS(e, iter(node, data)); + if ( searchmode == AKSL_TREE_SEARCH_BFS_RIGHT ) { + first = node->right; + second = node->left; + } else { + first = node->left; + second = node->right; + } + if ( first != NULL ) { + PASS(e, tree_queue_push(head, tail, lalloc, first, depth + 1)); + } + if ( second != NULL ) { + PASS(e, tree_queue_push(head, tail, lalloc, second, depth + 1)); + } + } + SUCCEED_RETURN(e); +} + +/* + * The depth-first recursion. + * + * Split out from aksl_tree_iterate so that AKERR_ITERATOR_BREAK *propagates*. + * This frame has no HANDLE block for it, so a break raised by the callback + * unwinds every frame up to the public entry point, which swallows it exactly + * once. In the old single-function form the frame that raised the break handled + * it and returned success, the parent's PASS therefore saw nothing wrong, and + * the walk carried on into the sibling subtree -- TODO.md 2.1.3. + * + * `path` is the chain of ancestors of `root` and `depth` its length. Checking + * each node against its own ancestry turns a tree that loops back on itself from + * infinite recursion into AKERR_CIRCULAR_REFERENCE, and the depth cap catches the + * degenerate chain that is merely too deep to recurse over (TODO.md 2.2.7). + */ +static akerr_ErrorContext AKERR_NOIGNORE *tree_dfs_walk( + aksl_TreeNode *root, + aksl_TreeNodeIterator iter, + uint8_t searchmode, + void *data, + aksl_TreeNode **path, + int depth) +{ + int i = 0; + PREPARE_ERROR(e); + FAIL_NONZERO_RETURN(e, (depth >= AKSL_TREE_MAX_DEPTH), AKERR_OUTOFBOUNDS, + "tree deeper than AKSL_TREE_MAX_DEPTH (%d) at node %p", + AKSL_TREE_MAX_DEPTH, (void *)root); + for ( i = 0; i < depth; i++ ) { + FAIL_NONZERO_RETURN(e, (path[i] == root), AKERR_CIRCULAR_REFERENCE, + "node %p is its own ancestor at depth %d", + (void *)root, depth); + } + path[depth] = root; + ATTEMPT { + switch ( searchmode ) { + case AKSL_TREE_SEARCH_DFS_PREORDER: + CATCH(e, iter(root, data)); + if ( root->left != NULL ) { + PASS(e, tree_dfs_walk(root->left, iter, searchmode, data, path, depth + 1)); + } + if ( root->right != NULL ) { + PASS(e, tree_dfs_walk(root->right, iter, searchmode, data, path, depth + 1)); + } + break; + case AKSL_TREE_SEARCH_DFS_POSTORDER: + if ( root->left != NULL ) { + PASS(e, tree_dfs_walk(root->left, iter, searchmode, data, path, depth + 1)); + } + if ( root->right != NULL ) { + PASS(e, tree_dfs_walk(root->right, iter, searchmode, data, path, depth + 1)); + } + CATCH(e, iter(root, data)); + break; + case AKSL_TREE_SEARCH_DFS_INORDER: + if ( root->left != NULL ) { + PASS(e, tree_dfs_walk(root->left, iter, searchmode, data, path, depth + 1)); + } + CATCH(e, iter(root, data)); + if ( root->right != NULL ) { + PASS(e, tree_dfs_walk(root->right, iter, searchmode, data, path, depth + 1)); + } + break; + case AKSL_TREE_SEARCH_VISIT: + /* Visit this node and stop; the children are deliberately not walked. */ + CATCH(e, iter(root, data)); + break; + } + } CLEANUP { + } PROCESS(e) { + } FINISH(e, true); + SUCCEED_RETURN(e); +} + /** - * @brief Iterates over a tree structure in breadth or depth first order, executing a callback function on each node. + * @brief Iterate a binary tree in breadth-first or depth-first order, calling a callback on each node. * - * This function recursively calls itself to traverse a binary tree datastructure. + * The callback may stop the walk early by raising AKERR_ITERATOR_BREAK; this + * function absorbs that one status and returns success, so an early exit is not + * an error to the caller. Any other status the callback raises propagates out + * unchanged, with its message and stack trace intact. * - * @param[in] root The tree structure to search - * @param[in] iter An aksl_TreeNodeIterator function which will be called for each node found - * @param[in] lalloc An aksl_AllocFunc function which will be used to allocate aksl_ListNode elements for the search, or NULL to use the default allocator (aksl_malloc) - * @param[in] lfree An aksl_FreeFunc function which will be used to free the elements procured with lalloc, or NULL to use the default free function (aksl_free) - * @param[in] searchmode One of the AKSL_TREE_SEARCH_BFS* defines - * @param[in] data Any user data that should be provided when the iterator is called - * @param[in] queue The linked list node to use as the head of the queue. The caller should pass NULL here. + * Traversal is bounded. A tree deeper than AKSL_TREE_MAX_DEPTH raises + * AKERR_OUTOFBOUNDS rather than overflowing the stack, and a depth-first walk + * over a tree whose child points back at one of its own ancestors raises + * AKERR_CIRCULAR_REFERENCE rather than recursing forever. * - * @throws AKERR_NULLPOINTER On null pointer inputs + * @param[in] root The root of the tree to walk + * @param[in] iter An aksl_TreeNodeIterator called once for each node visited + * @param[in] lalloc An aksl_AllocFunc used to allocate the internal traversal queue, or NULL for aksl_malloc. Breadth-first modes only; the depth-first modes allocate nothing. + * @param[in] lfree An aksl_FreeFunc used to release what lalloc returned, or NULL for aksl_free + * @param[in] searchmode One of the AKSL_TREE_SEARCH_* defines + * @param[in] data Caller data passed through to every invocation of iter + * + * @throws AKERR_NULLPOINTER On NULL root or iter + * @throws AKERR_VALUE On an unrecognised searchmode + * @throws AKERR_OUTOFBOUNDS When the tree is deeper than AKSL_TREE_MAX_DEPTH + * @throws AKERR_CIRCULAR_REFERENCE When a depth-first walk reaches a node that is its own ancestor * @return akerr_ErrorContext */ - akerr_ErrorContext AKERR_NOIGNORE *aksl_tree_iterate( aksl_TreeNode *root, aksl_TreeNodeIterator iter, aksl_AllocFunc lalloc, aksl_FreeFunc lfree, uint8_t searchmode, - void *data, - aksl_ListNode *queue) + void *data) { + TreeQueueEntry *head = NULL; + TreeQueueEntry *tail = NULL; + /* + * The ancestor chain for the depth-first walk, held here rather than spread + * across the recursion so that a walk costs one bounded allocation-free + * buffer no matter how the tree is shaped. + */ + aksl_TreeNode *path[AKSL_TREE_MAX_DEPTH]; PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, root, AKERR_NULLPOINTER, "root"); - FAIL_ZERO_RETURN(e, iter, AKERR_NULLPOINTER, "iter"); + FAIL_ZERO_RETURN(e, root, AKERR_NULLPOINTER, "root=%p, iter=%p", (void *)root, (void *)iter); + FAIL_ZERO_RETURN(e, iter, AKERR_NULLPOINTER, "root=%p, iter=%p", (void *)root, (void *)iter); if ( lalloc == NULL ) { lalloc = &aksl_malloc; } if ( lfree == NULL ) { lfree = &aksl_free; } + /* + * Validate the mode here, outside the ATTEMPT block, so that the walkers can + * assume it. The switch used to have no default at all, so searchmode 99 -- + * and AKSL_TREE_SEARCH_VISIT, which the header documented but nothing + * implemented -- fell straight through to success having visited nothing + * (TODO.md 2.2.9). + */ + switch ( searchmode ) { + case AKSL_TREE_SEARCH_DFS_PREORDER: + case AKSL_TREE_SEARCH_DFS_INORDER: + case AKSL_TREE_SEARCH_DFS_POSTORDER: + case AKSL_TREE_SEARCH_VISIT: + case AKSL_TREE_SEARCH_BFS: + case AKSL_TREE_SEARCH_BFS_RIGHT: + break; + default: + FAIL_RETURN(e, AKERR_VALUE, "unknown searchmode %d", searchmode); + } ATTEMPT { - switch ( searchmode ) { - case AKSL_TREE_SEARCH_DFS_PREORDER: - CATCH(e, iter(root, data)); - if ( root->left != NULL ) { - PASS(e, aksl_tree_iterate(root->left, iter, lalloc, lfree, searchmode, data, NULL)); - } - if ( root-> right != NULL ) { - PASS(e, aksl_tree_iterate(root->right, iter, lalloc, lfree, searchmode, data, NULL)); - } - break; - case AKSL_TREE_SEARCH_DFS_POSTORDER: - if ( root->left != NULL ) { - PASS(e, aksl_tree_iterate(root->left, iter, lalloc, lfree, searchmode, data, NULL)); - } - if ( root-> right != NULL ) { - PASS(e, aksl_tree_iterate(root->right, iter, lalloc, lfree, searchmode, data, NULL)); - } - CATCH(e, iter(root, data)); - break; - case AKSL_TREE_SEARCH_DFS_INORDER: - if ( root->left != NULL ) { - PASS(e, aksl_tree_iterate(root->left, iter, lalloc, lfree, searchmode, data, NULL)); - } - CATCH(e, iter(root, data)); - if ( root-> right != NULL ) { - PASS(e, aksl_tree_iterate(root->right, iter, lalloc, lfree, searchmode, data, NULL)); - } - break; - case AKSL_TREE_SEARCH_BFS: - case AKSL_TREE_SEARCH_BFS_RIGHT: - FAIL_RETURN(e, AKERR_NOT_IMPLEMENTED, "Searchmode %d", searchmode); - break; + if ( searchmode == AKSL_TREE_SEARCH_BFS || searchmode == AKSL_TREE_SEARCH_BFS_RIGHT ) { + CATCH(e, tree_queue_push(&head, &tail, lalloc, root, 0)); + CATCH(e, tree_bfs_walk(&head, &tail, iter, lalloc, lfree, searchmode, data)); + } else { + CATCH(e, tree_dfs_walk(root, iter, searchmode, data, path, 0)); } } CLEANUP { + /* + * Drains on every path, which is the whole reason the walk above leaves + * the queue alone when it fails. A failure to release the queue must not + * mask the error that got us here, so it is logged and dropped -- but + * dropped by hand rather than with IGNORE(), which logs the context and + * then never releases it back to the pool. + */ + akerr_ErrorContext *drained = tree_queue_drain(&head, &tail, lfree); + if ( drained != NULL ) { + LOG_ERROR_WITH_MESSAGE(drained, "** traversal queue drain failed **"); + drained = akerr_release_error(drained); + } } PROCESS(e) { } HANDLE(e, AKERR_ITERATOR_BREAK) { // This is not an error condition, it's just telling us to stop early @@ -379,17 +1229,23 @@ akerr_ErrorContext AKERR_NOIGNORE *aksl_list_iterate(aksl_ListNode *list, aksl_L FAIL_ZERO_RETURN(e, iter, AKERR_NULLPOINTER, "iter"); aksl_ListNode *slow = list; aksl_ListNode *fast = list; + aksl_ListNode *node = list; while ( fast != NULL && fast->next != NULL ) { slow = slow->next; fast = fast->next->next; if ( fast == slow) { - FAIL_RETURN(e, AKERR_CIRCULAR_REFERENCE, "%p", list); + FAIL_RETURN(e, AKERR_CIRCULAR_REFERENCE, "%p", (void *)list); } } - while ( slow != NULL ) { + /* + * From `list`, not from `slow`. The Floyd pass above leaves `slow` at the + * midpoint, and starting the visit there skipped the whole first half of the + * list including the head -- see TODO.md 2.1.2. + */ + while ( node != NULL ) { ATTEMPT { - CATCH(e, iter(slow, data)); - slow = slow->next; + CATCH(e, iter(node, data)); + node = node->next; } CLEANUP { } PROCESS(e) { } HANDLE(e, AKERR_ITERATOR_BREAK) { diff --git a/tests/test_convert.c b/tests/test_convert.c index 8e2636d..82422dd 100644 --- a/tests/test_convert.c +++ b/tests/test_convert.c @@ -1,19 +1,20 @@ /* * String -> number wrappers: aksl_atoi / atol / atoll / atof. * - * TODO.md section 1.4. This file covers the parts of the contract that are - * stable today: the happy paths (including negatives and leading whitespace) - * and the NULL guards. + * TODO.md section 1.4, complete. The cases that used to live in + * tests/test_convert_strict.c -- registered as a known failure because the ato* + * family had no error channel at all (2.1.5) -- are folded back in here now that + * they pass: non-numeric input, empty input, trailing junk and overflow are all + * errors rather than silent wrong answers. * - * It deliberately says nothing about non-numeric input, trailing junk or - * overflow. Those all return *success* today (TODO.md 2.1.5) and the correct - * behaviour is asserted by tests/test_convert_strict.c, which is registered as - * a known failure. Pinning the current lax behaviour here as well would mean - * this file starts failing on the day the defect is fixed. + * The strto* family these are built on is tested in tests/test_strto.c. */ #include "aksl_capture.h" +#include +#include + static int test_atoi_converts_positive(void) { int out = -1; @@ -46,9 +47,136 @@ static int test_atoi_rejects_null_arguments(void) int out = 0; AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_atoi(NULL, &out), - AKERR_NULLPOINTER, "nptr="); + AKERR_NULLPOINTER, "nptr="); AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_atoi("1", NULL), - AKERR_NULLPOINTER, "dest="); + AKERR_NULLPOINTER, "dest="); + return 0; +} + +/* + * The whole reason 2.1.5 mattered. atoi("not a number") returned success and 0, + * so a caller could not tell a parse failure from a legitimate zero -- and + * akbasic had to write ~60 lines of its own strtoll/strtod wrapper to avoid + * turning four diagnosable BASIC errors into four wrong answers. + */ +static int test_non_numeric_input_is_a_value_error(void) +{ + int out = 99; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_atoi("not a number", &out), + AKERR_VALUE, "no digits"); + /* *dest is 0 on every failure path, never left holding a partial answer. */ + AKSL_CHECK(out == 0); + return 0; +} + +static int test_empty_input_is_a_value_error(void) +{ + int out = 99; + + AKSL_CHECK_STATUS(aksl_atoi("", &out), AKERR_VALUE); + AKSL_CHECK(out == 0); + AKSL_CHECK_STATUS(aksl_atoi(" ", &out), AKERR_VALUE); + AKSL_CHECK(out == 0); + return 0; +} + +static int test_trailing_junk_is_a_value_error(void) +{ + int out = 99; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_atoi("12abc", &out), + AKERR_VALUE, "trailing junk"); + AKSL_CHECK(out == 0); + /* Trailing whitespace counts as junk too: the whole string must be a number. */ + AKSL_CHECK_STATUS(aksl_atoi("12 ", &out), AKERR_VALUE); + return 0; +} + +/* + * "0x10" through the ato* forms is base 10, so parsing stops at the 'x' and the + * rest is trailing junk. TODO.md 1.4 left this open; the answer is that the + * prefix-honouring parse is aksl_strtol(nptr, NULL, 0, &dest), and tests/ + * test_strto.c holds that half. + */ +static int test_hex_literal_is_rejected_by_the_base_10_forms(void) +{ + int out = 99; + + AKSL_CHECK_STATUS(aksl_atoi("0x10", &out), AKERR_VALUE); + AKSL_CHECK(out == 0); + return 0; +} + +static int test_overflow_is_erange(void) +{ + int out = 99; + long lout = 99; + long long llout = 99; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_atoi("99999999999999999999", &out), + ERANGE, "out of range"); + AKSL_CHECK(out == 0); + AKSL_CHECK_STATUS(aksl_atol("99999999999999999999999999", &lout), ERANGE); + AKSL_CHECK(lout == 0); + AKSL_CHECK_STATUS(aksl_atoll("99999999999999999999999999", &llout), ERANGE); + AKSL_CHECK(llout == 0); + return 0; +} + +/* + * aksl_atoi narrows from long to int, so a value that fits a long and not an int + * is ERANGE from the wrapper rather than from strtol. On a platform where long + * and int are the same width there is nothing to narrow and strtol has already + * caught it, so the case is skipped rather than asserted into a false pass. + */ +static int test_atoi_narrows_to_int_range(void) +{ +#if LONG_MAX > INT_MAX + int out = 99; + char buf[64]; + + snprintf(buf, sizeof(buf), "%ld", (long)INT_MAX + 1); + AKSL_CHECK_STATUS(aksl_atoi(buf, &out), ERANGE); + AKSL_CHECK(out == 0); + + snprintf(buf, sizeof(buf), "%ld", (long)INT_MIN - 1); + AKSL_CHECK_STATUS(aksl_atoi(buf, &out), ERANGE); + AKSL_CHECK(out == 0); +#else + fprintf(stderr, " (skipped: long and int are the same width here)\n"); +#endif + return 0; +} + +/* TODO.md 1.4: the type boundaries round-trip exactly rather than nearly. */ +static int test_type_boundaries_round_trip(void) +{ + char buf[64]; + int iout = 0; + long lout = 0; + long long llout = 0; + + snprintf(buf, sizeof(buf), "%d", INT_MAX); + AKSL_CHECK_OK(aksl_atoi(buf, &iout)); + AKSL_CHECK(iout == INT_MAX); + snprintf(buf, sizeof(buf), "%d", INT_MIN); + AKSL_CHECK_OK(aksl_atoi(buf, &iout)); + AKSL_CHECK(iout == INT_MIN); + + snprintf(buf, sizeof(buf), "%ld", LONG_MAX); + AKSL_CHECK_OK(aksl_atol(buf, &lout)); + AKSL_CHECK(lout == LONG_MAX); + snprintf(buf, sizeof(buf), "%ld", LONG_MIN); + AKSL_CHECK_OK(aksl_atol(buf, &lout)); + AKSL_CHECK(lout == LONG_MIN); + + snprintf(buf, sizeof(buf), "%lld", LLONG_MAX); + AKSL_CHECK_OK(aksl_atoll(buf, &llout)); + AKSL_CHECK(llout == LLONG_MAX); + snprintf(buf, sizeof(buf), "%lld", LLONG_MIN); + AKSL_CHECK_OK(aksl_atoll(buf, &llout)); + AKSL_CHECK(llout == LLONG_MIN); return 0; } @@ -63,6 +191,7 @@ static int test_atol_converts_and_rejects_null(void) AKSL_CHECK_STATUS(aksl_atol(NULL, &out), AKERR_NULLPOINTER); AKSL_CHECK_STATUS(aksl_atol("1", NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_atol("nope", &out), AKERR_VALUE); return 0; } @@ -77,6 +206,7 @@ static int test_atoll_converts_and_rejects_null(void) AKSL_CHECK_STATUS(aksl_atoll(NULL, &out), AKERR_NULLPOINTER); AKSL_CHECK_STATUS(aksl_atoll("1", NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_atoll("nope", &out), AKERR_VALUE); return 0; } @@ -88,9 +218,45 @@ static int test_atof_converts_and_rejects_null(void) AKSL_CHECK(out == 2.5); AKSL_CHECK_OK(aksl_atof(" -0.125", &out)); AKSL_CHECK(out == -0.125); + AKSL_CHECK_OK(aksl_atof("1e3", &out)); + AKSL_CHECK(out == 1000.0); AKSL_CHECK_STATUS(aksl_atof(NULL, &out), AKERR_NULLPOINTER); AKSL_CHECK_STATUS(aksl_atof("1", NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_atof("nope", &out), AKERR_VALUE); + AKSL_CHECK_STATUS(aksl_atof("1.5xyz", &out), AKERR_VALUE); + return 0; +} + +/* + * TODO.md 1.4 left "inf" and "nan" open. They are accepted, because strtod(3) + * accepts them and because they are exact round-trips rather than approximations + * of something else -- a caller who did not want them has a domain check to make + * that this library cannot make for it. + */ +static int test_atof_accepts_infinity_and_nan(void) +{ + double out = 0.0; + + AKSL_CHECK_OK(aksl_atof("inf", &out)); + AKSL_CHECK(out > 0.0 && (out * 2.0) == out); + AKSL_CHECK_OK(aksl_atof("-INFINITY", &out)); + AKSL_CHECK(out < 0.0 && (out * 2.0) == out); + AKSL_CHECK_OK(aksl_atof("nan", &out)); + /* The only value that is not equal to itself. */ + AKSL_CHECK(out != out); + return 0; +} + +/* Overflow and underflow both come back as ERANGE, which is strtod's only signal. */ +static int test_atof_range_errors(void) +{ + double out = 1.0; + + AKSL_CHECK_STATUS(aksl_atof("1e400", &out), ERANGE); + AKSL_CHECK(out == 0.0); + AKSL_CHECK_STATUS(aksl_atof("1e-400", &out), ERANGE); + AKSL_CHECK(out == 0.0); return 0; } @@ -107,6 +273,25 @@ static int test_conversions_are_repeatable(void) return 0; } +/* + * errno is cleared before each wrapped call, so a failure left over from an + * earlier unrelated call cannot be reported as this one's. Set errno to + * something conspicuous first and check that a successful conversion still + * succeeds -- the old code could report a stale errno as its own status. + */ +static int test_stale_errno_does_not_leak_into_the_result(void) +{ + int out = 0; + + errno = ERANGE; + AKSL_CHECK_OK(aksl_atoi("5", &out)); + AKSL_CHECK(out == 5); + + errno = ENOMEM; + AKSL_CHECK_STATUS(aksl_atoi("junk", &out), AKERR_VALUE); + return 0; +} + int main(void) { int failures = 0; @@ -118,11 +303,22 @@ int main(void) AKSL_RUN(failures, test_atoi_skips_leading_whitespace); AKSL_RUN(failures, test_atoi_rejects_null_arguments); + AKSL_RUN(failures, test_non_numeric_input_is_a_value_error); + AKSL_RUN(failures, test_empty_input_is_a_value_error); + AKSL_RUN(failures, test_trailing_junk_is_a_value_error); + AKSL_RUN(failures, test_hex_literal_is_rejected_by_the_base_10_forms); + AKSL_RUN(failures, test_overflow_is_erange); + AKSL_RUN(failures, test_atoi_narrows_to_int_range); + AKSL_RUN(failures, test_type_boundaries_round_trip); + AKSL_RUN(failures, test_atol_converts_and_rejects_null); AKSL_RUN(failures, test_atoll_converts_and_rejects_null); AKSL_RUN(failures, test_atof_converts_and_rejects_null); + AKSL_RUN(failures, test_atof_accepts_infinity_and_nan); + AKSL_RUN(failures, test_atof_range_errors); AKSL_RUN(failures, test_conversions_are_repeatable); + AKSL_RUN(failures, test_stale_errno_does_not_leak_into_the_result); AKSL_REPORT(failures); } diff --git a/tests/test_convert_strict.c b/tests/test_convert_strict.c deleted file mode 100644 index a67bbd0..0000000 --- a/tests/test_convert_strict.c +++ /dev/null @@ -1,75 +0,0 @@ -/* - * KNOWN FAILING -- TODO.md section 2.1.5 - * - * The ato* wrappers cannot report a conversion failure. atoi(3) and friends have - * no error channel at all: "not a number" converts to 0 and an overflowing - * literal converts to a wrapped value, and in both cases the wrapper hands back - * success. A library whose entire purpose is turning silent libc failures into - * error contexts should not be the one place a bad conversion passes unnoticed. - * - * This test asserts the contract the fix should provide -- reimplemented over - * strtol/strtoll/strtod with errno cleared, an endptr check for "no digits - * consumed" and "trailing junk", and a range check: - * - * no digits consumed / trailing junk -> AKERR_VALUE - * value out of range -> ERANGE - * - * Registered in AKSL_KNOWN_FAILING_TESTS (WILL_FAIL). When the wrappers get a - * real error channel, CTest reports this as unexpectedly passing -- move it into - * AKSL_TESTS then, and fold the cases into tests/test_convert.c. - */ - -#include "aksl_capture.h" - -#include - -static int test_non_numeric_input_is_a_value_error(void) -{ - int out = 0; - - AKSL_CHECK_STATUS(aksl_atoi("not a number", &out), AKERR_VALUE); - return 0; -} - -static int test_empty_input_is_a_value_error(void) -{ - int out = 0; - - AKSL_CHECK_STATUS(aksl_atoi("", &out), AKERR_VALUE); - AKSL_CHECK_STATUS(aksl_atoi(" ", &out), AKERR_VALUE); - return 0; -} - -static int test_trailing_junk_is_a_value_error(void) -{ - int out = 0; - - AKSL_CHECK_STATUS(aksl_atoi("12abc", &out), AKERR_VALUE); - return 0; -} - -static int test_overflow_is_erange(void) -{ - int out = 0; - long lout = 0; - long long llout = 0; - - AKSL_CHECK_STATUS(aksl_atoi("99999999999999999999", &out), ERANGE); - AKSL_CHECK_STATUS(aksl_atol("99999999999999999999999999", &lout), ERANGE); - AKSL_CHECK_STATUS(aksl_atoll("99999999999999999999999999", &llout), ERANGE); - return 0; -} - -int main(void) -{ - int failures = 0; - - akerr_init(); - - AKSL_RUN(failures, test_non_numeric_input_is_a_value_error); - AKSL_RUN(failures, test_empty_input_is_a_value_error); - AKSL_RUN(failures, test_trailing_junk_is_a_value_error); - AKSL_RUN(failures, test_overflow_is_erange); - - AKSL_REPORT(failures); -} diff --git a/tests/test_format.c b/tests/test_format.c index 9767aae..d5e2a8a 100644 --- a/tests/test_format.c +++ b/tests/test_format.c @@ -1,17 +1,19 @@ /* - * Formatted-output wrappers: aksl_printf / aksl_fprintf / aksl_sprintf. + * Formatted-output wrappers: aksl_printf / aksl_fprintf / aksl_snprintf and + * their va_list forms. * - * TODO.md section 1.3. Each happy path asserts both halves of the contract -- - * the byte count handed back through *count and the text that actually landed - * somewhere -- and every pointer argument is checked for its NULL guard. + * TODO.md section 1.3, now complete. Each happy path asserts both halves of the + * contract -- the byte count handed back through *count and the text that + * actually landed somewhere -- and every pointer argument is checked for its + * NULL guard. * * Readback goes through plain libc rather than aksl_fread so that a failure here * points at the formatted-output wrapper under test and not at the stream * wrappers, which tests/test_stream.c covers. * - * Not covered: the destination-overflow case, because aksl_sprintf wraps the - * unbounded vsprintf and there is no bounded entry point to test yet - * (TODO.md 2.2.4). + * aksl_sprintf is gone (TODO.md 2.2.4) and aksl_snprintf takes its place, so the + * destination-overflow case that could not previously be written is here: it is + * AKERR_OUTOFBOUNDS, not the short success snprintf(3) would have reported. */ #include "aksl_capture.h" @@ -37,41 +39,81 @@ static long read_file(const char *path, char *buf, size_t n) return (long)got; } -static int test_sprintf_writes_text_and_count(void) +static int test_snprintf_writes_text_and_count(void) { char buf[64]; int count = -1; memset(buf, 0x00, sizeof(buf)); - AKSL_CHECK_OK(aksl_sprintf(&count, buf, "%s=%d", "x", 7)); + AKSL_CHECK_OK(aksl_snprintf(&count, buf, sizeof(buf), "%s=%d", "x", 7)); AKSL_CHECK(count == 3); AKSL_CHECK(strcmp(buf, "x=7") == 0); return 0; } -static int test_sprintf_empty_format_writes_nothing(void) +static int test_snprintf_empty_format_writes_nothing(void) { char buf[8] = { 'z', 'z', 'z', 'z', 'z', 'z', 'z', 'z' }; int count = -1; - AKSL_CHECK_OK(aksl_sprintf(&count, buf, "%s", "")); + AKSL_CHECK_OK(aksl_snprintf(&count, buf, sizeof(buf), "%s", "")); AKSL_CHECK(count == 0); AKSL_CHECK(buf[0] == '\0'); return 0; } -static int test_sprintf_rejects_null_arguments(void) +/* + * The case that could not be written while the wrapper was aksl_sprintf: output + * longer than the destination. snprintf(3) would truncate, NUL-terminate and + * report the length it *would* have written, leaving the caller to notice; here + * it is an error, and *count is 0 rather than the would-have-been length. + */ +static int test_snprintf_truncation_is_an_error(void) +{ + char buf[8]; + int count = -1; + + memset(buf, 0x00, sizeof(buf)); + AKSL_CHECK_STATUS_MSG_CONTAINS( + aksl_snprintf(&count, buf, sizeof(buf), "%s", "far too long for eight bytes"), + AKERR_OUTOFBOUNDS, "truncated"); + AKSL_CHECK(count == 0); + return 0; +} + +/* Exactly filling the buffer is not truncation; one byte more is. */ +static int test_snprintf_boundary_is_exact(void) +{ + char buf[8]; + int count = -1; + + /* 7 characters plus the NUL is exactly sizeof(buf). */ + AKSL_CHECK_OK(aksl_snprintf(&count, buf, sizeof(buf), "%s", "1234567")); + AKSL_CHECK(count == 7); + AKSL_CHECK(strcmp(buf, "1234567") == 0); + + count = -1; + AKSL_CHECK_STATUS(aksl_snprintf(&count, buf, sizeof(buf), "%s", "12345678"), + AKERR_OUTOFBOUNDS); + AKSL_CHECK(count == 0); + return 0; +} + +static int test_snprintf_rejects_null_arguments_and_zero_size(void) { char buf[8]; int count = 0; memset(buf, 0x00, sizeof(buf)); - AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_sprintf(NULL, buf, "x"), - AKERR_NULLPOINTER, "count="); - AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_sprintf(&count, NULL, "x"), - AKERR_NULLPOINTER, "str="); - AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_sprintf(&count, buf, NULL), - AKERR_NULLPOINTER, "format="); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_snprintf(NULL, buf, sizeof(buf), "x"), + AKERR_NULLPOINTER, "count="); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_snprintf(&count, NULL, 8, "x"), + AKERR_NULLPOINTER, "str="); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_snprintf(&count, buf, sizeof(buf), NULL), + AKERR_NULLPOINTER, "format="); + /* size 0 leaves no room even for the terminator, so there is nothing to do. */ + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_snprintf(&count, buf, 0, "x"), + AKERR_VALUE, "size=0"); return 0; } @@ -96,19 +138,21 @@ static int test_fprintf_writes_to_stream(void) /* * vfprintf on a stream opened "r" fails outright, so the wrapper reports the - * errno it saw (EBADF on glibc). *count is left holding -1 in this case, which - * TODO.md 1.3 flags as a contract gap -- the status is the assertion here. + * errno it saw (EBADF on glibc). *count is 0 afterwards, not vfprintf's -1: + * TODO.md 1.3 recorded the negative count as a contract gap, and this is the + * assertion that closes it. */ static int test_fprintf_to_read_only_stream_reports_errno(void) { char path[AKSL_TMP_MAX]; FILE *fp = NULL; - int count = 0; + int count = 99; AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0); AKSL_CHECK_OK(aksl_fopen(path, "r", &fp)); AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fprintf(&count, fp, "%d", 1), - EBADF, "Short write"); + EBADF, "Short write"); + AKSL_CHECK(count == 0); AKSL_CHECK_OK(aksl_fclose(fp)); AKSL_CHECK(unlink(path) == 0); return 0; @@ -119,11 +163,11 @@ static int test_fprintf_rejects_null_arguments(void) int count = 0; AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fprintf(NULL, stdout, "x"), - AKERR_NULLPOINTER, "count="); + AKERR_NULLPOINTER, "count="); AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fprintf(&count, NULL, "x"), - AKERR_NULLPOINTER, "stream="); + AKERR_NULLPOINTER, "stream="); AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_fprintf(&count, stdout, NULL), - AKERR_NULLPOINTER, "format="); + AKERR_NULLPOINTER, "format="); return 0; } @@ -174,9 +218,43 @@ static int test_printf_rejects_null_arguments(void) int count = 0; AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_printf(NULL, "x"), - AKERR_NULLPOINTER, "count="); + AKERR_NULLPOINTER, "count="); AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_printf(&count, NULL), - AKERR_NULLPOINTER, "format="); + AKERR_NULLPOINTER, "format="); + return 0; +} + +/* + * The va_list forms are what the variadic ones are built on, and TODO.md 3.1 + * wanted them exposed so consumers can write their own variadic wrappers. This + * is a consumer doing exactly that. + */ +static akerr_ErrorContext AKERR_NOIGNORE *consumer_wrapper(int *count, char *buf, size_t n, + const char *fmt, ...) +{ + va_list args; + akerr_ErrorContext *raised = NULL; + + va_start(args, fmt); + raised = aksl_vsnprintf(count, buf, n, fmt, args); + va_end(args); + return raised; +} + +static int test_va_list_forms_are_usable_from_outside(void) +{ + char buf[32]; + int count = -1; + + memset(buf, 0x00, sizeof(buf)); + AKSL_CHECK_OK(consumer_wrapper(&count, buf, sizeof(buf), "%s/%d", "via", 3)); + AKSL_CHECK(count == 5); + AKSL_CHECK(strcmp(buf, "via/3") == 0); + + /* The error contract survives the extra layer intact. */ + AKSL_CHECK_STATUS(consumer_wrapper(&count, buf, 4, "%s", "too long"), + AKERR_OUTOFBOUNDS); + AKSL_CHECK(count == 0); return 0; } @@ -193,8 +271,8 @@ static int test_variadic_wrappers_survive_repeated_calls(void) int i = 0; for ( i = 0; i < 512; i++ ) { - AKSL_CHECK_OK(aksl_sprintf(&count, buf, "%d %s %ld %c %f", - i, "iteration", (long)i, 'x', (double)i)); + AKSL_CHECK_OK(aksl_snprintf(&count, buf, sizeof(buf), "%d %s %ld %c %f", + i, "iteration", (long)i, 'x', (double)i)); AKSL_CHECK(count > 0); AKSL_CHECK((size_t)count == strlen(buf)); } @@ -207,9 +285,11 @@ int main(void) akerr_init(); - AKSL_RUN(failures, test_sprintf_writes_text_and_count); - AKSL_RUN(failures, test_sprintf_empty_format_writes_nothing); - AKSL_RUN(failures, test_sprintf_rejects_null_arguments); + AKSL_RUN(failures, test_snprintf_writes_text_and_count); + AKSL_RUN(failures, test_snprintf_empty_format_writes_nothing); + AKSL_RUN(failures, test_snprintf_truncation_is_an_error); + AKSL_RUN(failures, test_snprintf_boundary_is_exact); + AKSL_RUN(failures, test_snprintf_rejects_null_arguments_and_zero_size); AKSL_RUN(failures, test_fprintf_writes_to_stream); AKSL_RUN(failures, test_fprintf_to_read_only_stream_reports_errno); @@ -218,6 +298,7 @@ int main(void) AKSL_RUN(failures, test_printf_writes_to_stdout); AKSL_RUN(failures, test_printf_rejects_null_arguments); + AKSL_RUN(failures, test_va_list_forms_are_usable_from_outside); AKSL_RUN(failures, test_variadic_wrappers_survive_repeated_calls); AKSL_REPORT(failures); diff --git a/tests/test_linkedlist.c b/tests/test_linkedlist.c index be44b6b..4a74cda 100644 --- a/tests/test_linkedlist.c +++ b/tests/test_linkedlist.c @@ -1,20 +1,20 @@ /* - * Linked-list behaviour that the library gets right today. + * Linked list -- TODO.md section 1.7, complete. * - * The two confirmed list defects (TODO.md 2.1.1 aksl_list_append truncating the - * chain, and 2.1.2 aksl_list_iterate skipping the head) are asserted separately - * in tests/test_list_append_chain.c and tests/test_list_iterate_head.c, which - * are registered as known-failing. Everything in this file must pass. + * The two confirmed list defects are fixed, so the tests that used to live in + * tests/test_list_append_chain.c and tests/test_list_iterate_head.c are folded + * back in here: aksl_list_append builds the whole chain rather than truncating + * it at the midpoint (2.1.1), and aksl_list_iterate starts at the head rather + * than at whatever node Floyd's slow pointer happened to stop on (2.1.2). * - * Note that the list-shape assertions here build their lists by hand rather - * than with aksl_list_append: append cannot be trusted to produce a chain - * longer than two nodes until 2.1.1 is fixed, and using it would make these - * tests fail for a reason that has nothing to do with what they are checking. + * The chain assertions build their lists with aksl_list_append now, which is the + * point -- they could not, while append was the thing under suspicion. */ #include "aksl_capture.h" #define MAX_VISITS 16 +#define CHAIN_LEN 5 typedef struct VisitLog { @@ -54,6 +54,31 @@ static akerr_ErrorContext AKERR_NOIGNORE *record_visit(aksl_ListNode *node, void SUCCEED_RETURN(e); } +/* ---------------------------------------------------------------------- */ +/* aksl_list_node_init */ +/* ---------------------------------------------------------------------- */ + +/* + * TODO.md 2.2.14: every caller used to have to remember to memset a node before + * its first use, and a stack node that skipped it walked straight into garbage. + */ +static int test_node_init_zeroes_the_links(void) +{ + aksl_ListNode node; + int payload = 7; + + memset((void *)&node, 0xff, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, &payload)); + AKSL_CHECK(node.next == NULL); + AKSL_CHECK(node.prev == NULL); + AKSL_CHECK(node.data == (void *)&payload); + + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); + AKSL_CHECK(node.data == NULL); + AKSL_CHECK_STATUS(aksl_list_node_init(NULL, NULL), AKERR_NULLPOINTER); + return 0; +} + /* ---------------------------------------------------------------------- */ /* aksl_list_append */ /* ---------------------------------------------------------------------- */ @@ -63,8 +88,8 @@ static int test_append_single_node(void) aksl_ListNode head; aksl_ListNode tail; - memset((void *)&head, 0x00, sizeof(head)); - memset((void *)&tail, 0x00, sizeof(tail)); + AKSL_CHECK_OK(aksl_list_node_init(&head, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&tail, NULL)); AKSL_CHECK_OK(aksl_list_append(&head, &tail)); AKSL_CHECK(head.next == &tail); @@ -74,11 +99,66 @@ static int test_append_single_node(void) return 0; } +/* + * The defect that made this library's list unusable: `tail` was assigned from + * Floyd's `slow` cursor *before* slow advanced, so it tracked the node behind + * the midpoint rather than the last node. Appending n1..n4 to n0 produced the + * chain "n0 -> n4" and silently dropped n1, n2 and n3. TODO.md 2.1.1. + */ +static int test_append_builds_the_whole_chain(void) +{ + aksl_ListNode node[CHAIN_LEN]; + aksl_ListNode *walk = NULL; + int i = 0; + + for ( i = 0; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + } + + /* Forward links, head to tail. */ + walk = &node[0]; + for ( i = 0; i < CHAIN_LEN; i++ ) { + AKSL_CHECK(walk == &node[i]); + walk = walk->next; + } + AKSL_CHECK(walk == NULL); + + /* Back links, tail to head. */ + walk = &node[CHAIN_LEN - 1]; + for ( i = CHAIN_LEN - 1; i >= 0; i-- ) { + AKSL_CHECK(walk == &node[i]); + walk = walk->prev; + } + AKSL_CHECK(walk == NULL); + return 0; +} + +/* append points obj->prev at the real tail and terminates the chain at obj. */ +static int test_append_links_prev_to_the_real_tail(void) +{ + aksl_ListNode node[4]; + int i = 0; + + for ( i = 0; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + AKSL_CHECK(node[i].prev == &node[i - 1]); + AKSL_CHECK(node[i].next == NULL); + AKSL_CHECK(node[i - 1].next == &node[i]); + } + return 0; +} + static int test_append_null_arguments(void) { aksl_ListNode node; - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); AKSL_CHECK_STATUS(aksl_list_append(NULL, &node), AKERR_NULLPOINTER); AKSL_CHECK_STATUS(aksl_list_append(&node, NULL), AKERR_NULLPOINTER); @@ -90,8 +170,8 @@ static int test_append_detects_self_cycle(void) aksl_ListNode head; aksl_ListNode node; - memset((void *)&head, 0x00, sizeof(head)); - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&head, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); head.next = &head; AKSL_CHECK_STATUS(aksl_list_append(&head, &node), AKERR_CIRCULAR_REFERENCE); @@ -104,9 +184,9 @@ static int test_append_detects_two_node_cycle(void) aksl_ListNode b; aksl_ListNode node; - memset((void *)&a, 0x00, sizeof(a)); - memset((void *)&b, 0x00, sizeof(b)); - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); a.next = &b; b.prev = &a; b.next = &a; @@ -115,6 +195,56 @@ static int test_append_detects_two_node_cycle(void) return 0; } +/* A cycle that does not include the head: a -> b -> c -> b. */ +static int test_append_detects_cycle_below_the_head(void) +{ + aksl_ListNode a; + aksl_ListNode b; + aksl_ListNode c; + aksl_ListNode node; + + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&c, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); + a.next = &b; + b.prev = &a; + b.next = &c; + c.prev = &b; + c.next = &b; + + AKSL_CHECK_STATUS(aksl_list_append(&a, &node), AKERR_CIRCULAR_REFERENCE); + return 0; +} + +/* + * TODO.md 1.7 asked for the aliasing contract to be defined. It is refusal: + * relinking a node that is already in the list would orphan everything between + * its old position and the tail, so the tail walk -- which happens anyway -- + * doubles as the check. + */ +static int test_append_refuses_a_node_already_in_the_list(void) +{ + aksl_ListNode node[3]; + int i = 0; + + for ( i = 0; i < 3; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[1])); + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[2])); + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_list_append(&node[0], &node[1]), + AKERR_VALUE, "already in this list"); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_list_append(&node[0], &node[0]), + AKERR_VALUE, "already the head"); + /* The list is untouched by the refusal. */ + AKSL_CHECK(node[0].next == &node[1]); + AKSL_CHECK(node[1].next == &node[2]); + AKSL_CHECK(node[2].next == NULL); + return 0; +} + /* ---------------------------------------------------------------------- */ /* aksl_list_iterate */ /* ---------------------------------------------------------------------- */ @@ -124,7 +254,7 @@ static int test_iterate_null_arguments(void) aksl_ListNode node; VisitLog log; - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); visitlog_init(&log); AKSL_CHECK_STATUS(aksl_list_iterate(NULL, &record_visit, &log), AKERR_NULLPOINTER); @@ -138,7 +268,7 @@ static int test_iterate_single_node(void) aksl_ListNode node; VisitLog log; - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); visitlog_init(&log); AKSL_CHECK_OK(aksl_list_iterate(&node, &record_visit, &log)); @@ -147,17 +277,69 @@ static int test_iterate_single_node(void) return 0; } +/* + * Every node, exactly once, in order, starting at the head. The cycle check + * leaves Floyd's `slow` cursor at the list midpoint, and the visiting loop used + * to start from there -- so the whole first half of the list, head included, was + * never passed to the callback at all. TODO.md 2.1.2. + */ +static int test_iterate_visits_every_node_from_the_head(void) +{ + aksl_ListNode node[CHAIN_LEN]; + VisitLog log; + int i = 0; + + for ( i = 0; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + } + visitlog_init(&log); + + AKSL_CHECK_OK(aksl_list_iterate(&node[0], &record_visit, &log)); + AKSL_CHECK(log.count == CHAIN_LEN); + for ( i = 0; i < CHAIN_LEN; i++ ) { + AKSL_CHECK(log.seen[i] == &node[i]); + } + return 0; +} + +/* An even-length list too: Floyd's midpoint lands differently, the answer does not. */ +static int test_iterate_visits_every_node_of_an_even_list(void) +{ + aksl_ListNode node[4]; + VisitLog log; + int i = 0; + + for ( i = 0; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + } + visitlog_init(&log); + + AKSL_CHECK_OK(aksl_list_iterate(&node[0], &record_visit, &log)); + AKSL_CHECK(log.count == 4); + for ( i = 0; i < 4; i++ ) { + AKSL_CHECK(log.seen[i] == &node[i]); + } + return 0; +} + static int test_iterate_detects_self_cycle(void) { aksl_ListNode head; VisitLog log; - memset((void *)&head, 0x00, sizeof(head)); + AKSL_CHECK_OK(aksl_list_node_init(&head, NULL)); head.next = &head; visitlog_init(&log); AKSL_CHECK_STATUS(aksl_list_iterate(&head, &record_visit, &log), AKERR_CIRCULAR_REFERENCE); + AKSL_CHECK(log.count == 0); return 0; } @@ -167,8 +349,8 @@ static int test_iterate_detects_two_node_cycle(void) aksl_ListNode b; VisitLog log; - memset((void *)&a, 0x00, sizeof(a)); - memset((void *)&b, 0x00, sizeof(b)); + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); a.next = &b; b.prev = &a; b.next = &a; @@ -179,6 +361,29 @@ static int test_iterate_detects_two_node_cycle(void) return 0; } +/* Tail pointing back into the middle: a -> b -> c -> b. */ +static int test_iterate_detects_tail_to_middle_cycle(void) +{ + aksl_ListNode a; + aksl_ListNode b; + aksl_ListNode c; + VisitLog log; + + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&c, NULL)); + a.next = &b; + b.prev = &a; + b.next = &c; + c.prev = &b; + c.next = &b; + visitlog_init(&log); + + AKSL_CHECK_STATUS(aksl_list_iterate(&a, &record_visit, &log), + AKERR_CIRCULAR_REFERENCE); + return 0; +} + /* An error other than ITERATOR_BREAK must come back out of the iteration with * its status and message intact. */ static int test_iterate_propagates_callback_error(void) @@ -186,13 +391,13 @@ static int test_iterate_propagates_callback_error(void) aksl_ListNode node; VisitLog log; - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); visitlog_init(&log); log.fail_at = 0; AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_list_iterate(&node, &record_visit, &log), - AKERR_VALUE, "iterator failed at visit 0"); + aksl_list_iterate(&node, &record_visit, &log), + AKERR_VALUE, "iterator failed at visit 0"); AKSL_CHECK(log.count == 1); return 0; } @@ -203,7 +408,7 @@ static int test_iterate_break_is_not_an_error(void) aksl_ListNode node; VisitLog log; - memset((void *)&node, 0x00, sizeof(node)); + AKSL_CHECK_OK(aksl_list_node_init(&node, NULL)); visitlog_init(&log); log.break_at = 0; @@ -212,6 +417,32 @@ static int test_iterate_break_is_not_an_error(void) return 0; } +/* + * And the count is what proves it stopped early rather than merely finishing. + * A break on the second of five nodes must leave three nodes unvisited. + */ +static int test_iterate_break_stops_at_that_node(void) +{ + aksl_ListNode node[CHAIN_LEN]; + VisitLog log; + int i = 0; + + for ( i = 0; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < CHAIN_LEN; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + } + visitlog_init(&log); + log.break_at = 1; + + AKSL_CHECK_OK(aksl_list_iterate(&node[0], &record_visit, &log)); + AKSL_CHECK(log.count == 2); + AKSL_CHECK(log.seen[0] == &node[0]); + AKSL_CHECK(log.seen[1] == &node[1]); + return 0; +} + /* ---------------------------------------------------------------------- */ /* aksl_list_pop */ /* ---------------------------------------------------------------------- */ @@ -221,16 +452,16 @@ static int test_pop_middle_node(void) aksl_ListNode a; aksl_ListNode b; aksl_ListNode c; + aksl_ListNode *head = &a; - memset((void *)&a, 0x00, sizeof(a)); - memset((void *)&b, 0x00, sizeof(b)); - memset((void *)&c, 0x00, sizeof(c)); - a.next = &b; - b.prev = &a; - b.next = &c; - c.prev = &b; + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&c, NULL)); + AKSL_CHECK_OK(aksl_list_append(&a, &b)); + AKSL_CHECK_OK(aksl_list_append(&a, &c)); - AKSL_CHECK_OK(aksl_list_pop(&b)); + AKSL_CHECK_OK(aksl_list_pop(&head, &b)); + AKSL_CHECK(head == &a); AKSL_CHECK(a.next == &c); AKSL_CHECK(c.prev == &a); AKSL_CHECK(b.next == NULL); @@ -238,17 +469,23 @@ static int test_pop_middle_node(void) return 0; } -static int test_pop_head_node(void) +/* + * TODO.md 2.2.12: popping the head used to leave the caller's own head pointer + * aimed at a node that was no longer in the list, with no way to learn the new + * one. That is what the head out-param is for, and this is the assertion. + */ +static int test_pop_head_node_moves_the_head(void) { aksl_ListNode a; aksl_ListNode b; + aksl_ListNode *head = &a; - memset((void *)&a, 0x00, sizeof(a)); - memset((void *)&b, 0x00, sizeof(b)); - a.next = &b; - b.prev = &a; + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_append(&a, &b)); - AKSL_CHECK_OK(aksl_list_pop(&a)); + AKSL_CHECK_OK(aksl_list_pop(&head, &a)); + AKSL_CHECK(head == &b); AKSL_CHECK(b.prev == NULL); AKSL_CHECK(b.next == NULL); AKSL_CHECK(a.next == NULL); @@ -260,13 +497,14 @@ static int test_pop_tail_node(void) { aksl_ListNode a; aksl_ListNode b; + aksl_ListNode *head = &a; - memset((void *)&a, 0x00, sizeof(a)); - memset((void *)&b, 0x00, sizeof(b)); - a.next = &b; - b.prev = &a; + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + AKSL_CHECK_OK(aksl_list_append(&a, &b)); - AKSL_CHECK_OK(aksl_list_pop(&b)); + AKSL_CHECK_OK(aksl_list_pop(&head, &b)); + AKSL_CHECK(head == &a); AKSL_CHECK(a.next == NULL); AKSL_CHECK(a.prev == NULL); AKSL_CHECK(b.next == NULL); @@ -274,21 +512,93 @@ static int test_pop_tail_node(void) return 0; } -static int test_pop_only_node(void) +/* Popping the only node empties the list, and the head becomes NULL. */ +static int test_pop_only_node_empties_the_list(void) { aksl_ListNode a; + aksl_ListNode *head = &a; - memset((void *)&a, 0x00, sizeof(a)); + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); - AKSL_CHECK_OK(aksl_list_pop(&a)); + AKSL_CHECK_OK(aksl_list_pop(&head, &a)); + AKSL_CHECK(head == NULL); AKSL_CHECK(a.next == NULL); AKSL_CHECK(a.prev == NULL); return 0; } -static int test_pop_null_argument(void) +static int test_pop_null_arguments(void) { - AKSL_CHECK_STATUS(aksl_list_pop(NULL), AKERR_NULLPOINTER); + aksl_ListNode a; + aksl_ListNode *head = &a; + + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_STATUS(aksl_list_pop(NULL, &a), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_list_pop(&head, NULL), AKERR_NULLPOINTER); + return 0; +} + +/* The list is still walkable after a pop, and the popped node is gone from it. */ +static int test_pop_then_iterate(void) +{ + aksl_ListNode node[4]; + aksl_ListNode *head = &node[0]; + VisitLog log; + int i = 0; + + for ( i = 0; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_node_init(&node[i], NULL)); + } + for ( i = 1; i < 4; i++ ) { + AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); + } + + AKSL_CHECK_OK(aksl_list_pop(&head, &node[2])); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_list_iterate(head, &record_visit, &log)); + AKSL_CHECK(log.count == 3); + AKSL_CHECK(log.seen[0] == &node[0]); + AKSL_CHECK(log.seen[1] == &node[1]); + AKSL_CHECK(log.seen[2] == &node[3]); + + /* And again, this time taking the head out. */ + AKSL_CHECK_OK(aksl_list_pop(&head, &node[0])); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_list_iterate(head, &record_visit, &log)); + AKSL_CHECK(log.count == 2); + AKSL_CHECK(log.seen[0] == &node[1]); + AKSL_CHECK(log.seen[1] == &node[3]); + return 0; +} + +/* + * TODO.md 1.7's pool-accounting case. AKSL_RUN already asserts that each test + * leaves the pool as it found it; this one drives enough failures in a row to + * exhaust the pool several times over, which is where a wrapper that raises an + * error and forgets to release it shows up as an outright exhaustion rather than + * as a slow leak. + */ +static int test_error_pool_survives_a_long_run_of_failures(void) +{ + aksl_ListNode a; + aksl_ListNode b; + aksl_ListNode *head = &a; + VisitLog log; + int i = 0; + + AKSL_CHECK_OK(aksl_list_node_init(&a, NULL)); + AKSL_CHECK_OK(aksl_list_node_init(&b, NULL)); + a.next = &a; + + for ( i = 0; i < AKERR_MAX_ARRAY_ERROR + 10; i++ ) { + AKSL_CHECK_STATUS(aksl_list_append(&a, &b), AKERR_CIRCULAR_REFERENCE); + AKSL_CHECK_STATUS(aksl_list_append(NULL, &b), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_list_pop(&head, NULL), AKERR_NULLPOINTER); + visitlog_init(&log); + AKSL_CHECK_STATUS(aksl_list_iterate(&a, &record_visit, &log), + AKERR_CIRCULAR_REFERENCE); + AKSL_CHECK(aksl_slots_in_use() == 0); + } return 0; } @@ -298,23 +608,36 @@ int main(void) akerr_init(); + AKSL_RUN(failures, test_node_init_zeroes_the_links); + AKSL_RUN(failures, test_append_single_node); + AKSL_RUN(failures, test_append_builds_the_whole_chain); + AKSL_RUN(failures, test_append_links_prev_to_the_real_tail); AKSL_RUN(failures, test_append_null_arguments); AKSL_RUN(failures, test_append_detects_self_cycle); AKSL_RUN(failures, test_append_detects_two_node_cycle); + AKSL_RUN(failures, test_append_detects_cycle_below_the_head); + AKSL_RUN(failures, test_append_refuses_a_node_already_in_the_list); AKSL_RUN(failures, test_iterate_null_arguments); AKSL_RUN(failures, test_iterate_single_node); + AKSL_RUN(failures, test_iterate_visits_every_node_from_the_head); + AKSL_RUN(failures, test_iterate_visits_every_node_of_an_even_list); AKSL_RUN(failures, test_iterate_detects_self_cycle); AKSL_RUN(failures, test_iterate_detects_two_node_cycle); + AKSL_RUN(failures, test_iterate_detects_tail_to_middle_cycle); AKSL_RUN(failures, test_iterate_propagates_callback_error); AKSL_RUN(failures, test_iterate_break_is_not_an_error); + AKSL_RUN(failures, test_iterate_break_stops_at_that_node); AKSL_RUN(failures, test_pop_middle_node); - AKSL_RUN(failures, test_pop_head_node); + AKSL_RUN(failures, test_pop_head_node_moves_the_head); AKSL_RUN(failures, test_pop_tail_node); - AKSL_RUN(failures, test_pop_only_node); - AKSL_RUN(failures, test_pop_null_argument); + AKSL_RUN(failures, test_pop_only_node_empties_the_list); + AKSL_RUN(failures, test_pop_null_arguments); + AKSL_RUN(failures, test_pop_then_iterate); + + AKSL_RUN(failures, test_error_pool_survives_a_long_run_of_failures); AKSL_REPORT(failures); } diff --git a/tests/test_list_append_chain.c b/tests/test_list_append_chain.c deleted file mode 100644 index 0e706ae..0000000 --- a/tests/test_list_append_chain.c +++ /dev/null @@ -1,59 +0,0 @@ -/* - * KNOWN FAILING -- TODO.md section 2.1.1 - * - * aksl_list_append conflates Floyd cycle detection with finding the tail: the - * `tail` cursor is assigned from `slow` *before* `slow` advances, so it tracks - * the node behind the list midpoint rather than the last node. Appending to any - * list of two or more nodes therefore overwrites an interior link and silently - * drops every node after it. - * - * Appending n1..n4 to n0 produces the chain "n0 -> n4"; this test asserts the - * correct "n0 -> n1 -> n2 -> n3 -> n4" and so fails until append is fixed. - * It is registered in AKSL_KNOWN_FAILING_TESTS, which marks it WILL_FAIL; when - * the fix lands, CTest will report it as unexpectedly passing, which is the - * signal to move it into AKSL_TESTS. - */ - -#include "aksl_capture.h" - -#define CHAIN_LEN 5 - -static int test_append_builds_full_chain(void) -{ - aksl_ListNode node[CHAIN_LEN]; - aksl_ListNode *walk = NULL; - int i = 0; - - memset((void *)node, 0x00, sizeof(node)); - - for ( i = 1; i < CHAIN_LEN; i++ ) { - AKSL_CHECK_OK(aksl_list_append(&node[0], &node[i])); - } - - /* Forward links, head to tail. */ - walk = &node[0]; - for ( i = 0; i < CHAIN_LEN; i++ ) { - AKSL_CHECK(walk == &node[i]); - walk = walk->next; - } - AKSL_CHECK(walk == NULL); - - /* Back links, tail to head. */ - walk = &node[CHAIN_LEN - 1]; - for ( i = CHAIN_LEN - 1; i >= 0; i-- ) { - AKSL_CHECK(walk == &node[i]); - walk = walk->prev; - } - AKSL_CHECK(walk == NULL); - - return 0; -} - -int main(void) -{ - int failures = 0; - - akerr_init(); - AKSL_RUN(failures, test_append_builds_full_chain); - AKSL_REPORT(failures); -} diff --git a/tests/test_list_iterate_head.c b/tests/test_list_iterate_head.c deleted file mode 100644 index 7a7cb62..0000000 --- a/tests/test_list_iterate_head.c +++ /dev/null @@ -1,71 +0,0 @@ -/* - * KNOWN FAILING -- TODO.md section 2.1.2 - * - * aksl_list_iterate runs a Floyd cycle check that leaves `slow` sitting at the - * list midpoint, and then starts the visiting loop from `slow` instead of from - * `list`. Every node before the midpoint -- including the head -- is never - * passed to the callback. - * - * The list here is built by hand rather than with aksl_list_append so that this - * test fails only for the iterate defect, not for the separate append defect in - * TODO.md 2.1.1. - * - * Registered in AKSL_KNOWN_FAILING_TESTS (WILL_FAIL). When iterate is fixed, - * CTest reports this as unexpectedly passing -- move it into AKSL_TESTS then. - */ - -#include "aksl_capture.h" - -#define CHAIN_LEN 3 - -typedef struct VisitLog -{ - int count; - aksl_ListNode *seen[CHAIN_LEN]; -} VisitLog; - -static akerr_ErrorContext AKERR_NOIGNORE *record_visit(aksl_ListNode *node, void *data) -{ - VisitLog *log = NULL; - - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); - FAIL_ZERO_RETURN(e, data, AKERR_NULLPOINTER, "data"); - log = (VisitLog *)data; - if ( log->count < CHAIN_LEN ) { - log->seen[log->count] = node; - } - log->count += 1; - SUCCEED_RETURN(e); -} - -static int test_iterate_visits_every_node_from_the_head(void) -{ - aksl_ListNode node[CHAIN_LEN]; - VisitLog log; - int i = 0; - - memset((void *)node, 0x00, sizeof(node)); - memset((void *)&log, 0x00, sizeof(log)); - - for ( i = 1; i < CHAIN_LEN; i++ ) { - node[i - 1].next = &node[i]; - node[i].prev = &node[i - 1]; - } - - AKSL_CHECK_OK(aksl_list_iterate(&node[0], &record_visit, &log)); - AKSL_CHECK(log.count == CHAIN_LEN); - for ( i = 0; i < CHAIN_LEN; i++ ) { - AKSL_CHECK(log.seen[i] == &node[i]); - } - return 0; -} - -int main(void) -{ - int failures = 0; - - akerr_init(); - AKSL_RUN(failures, test_iterate_visits_every_node_from_the_head); - AKSL_REPORT(failures); -} diff --git a/tests/test_memory.c b/tests/test_memory.c index fce15e1..e272300 100644 --- a/tests/test_memory.c +++ b/tests/test_memory.c @@ -1,15 +1,19 @@ /* - * Memory-wrapper behaviour that is deterministic today. + * Memory wrappers -- TODO.md section 1.1, now complete, plus the additions from + * section 3.1. * - * TODO.md section 1.1 also calls out malloc(0), malloc(SIZE_MAX), and - * overlapping memcpy. Those are intentionally not pinned here yet: malloc(0) - * is platform-dependent under the current wrapper, huge allocations need - * sanitizer-safe handling, and overlapping memcpy is still an API-contract - * decision. + * The three cases 1.1 left open are all pinned here: malloc(0) is AKERR_VALUE + * rather than whatever errno happened to hold when the platform's malloc(0) + * returned NULL; an allocation the system cannot satisfy reports ENOMEM and + * leaves *dst NULL rather than garbage; and overlapping memcpy is refused with + * AKERR_VALUE rather than left as undefined behaviour that usually looks fine. */ #include "aksl_capture.h" +#include +#include + static int test_malloc_writes_nonnull_pointer(void) { void *ptr = NULL; @@ -23,14 +27,119 @@ static int test_malloc_writes_nonnull_pointer(void) static int test_malloc_rejects_null_destination(void) { AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_malloc(32, NULL), - AKERR_NULLPOINTER, "NULL"); + AKERR_NULLPOINTER, "dst="); + return 0; +} + +/* + * TODO.md 1.1 asked for this contract to be pinned down. malloc(0) is allowed to + * return either a unique pointer or NULL, and a NULL there is not a failure and + * need not set errno -- so the old wrapper could raise an error whose status was + * 0, which every DETECT downstream reads as success while the context holds a + * pool slot. A zero-size request is refused on its own terms instead. + */ +static int test_malloc_rejects_zero_size(void) +{ + void *ptr = (void *)0x1; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_malloc(0, &ptr), AKERR_VALUE, "zero-size"); + AKSL_CHECK(ptr == NULL); + return 0; +} + +/* + * An allocation no system can satisfy. SIZE_MAX/2 rather than SIZE_MAX because + * some allocators reject the latter as an obvious overflow before ever + * consulting the system, and the case worth testing is the one where the request + * is plausible and the answer is still no. + */ +static int test_malloc_reports_out_of_memory(void) +{ + void *ptr = (void *)0x1; + + AKSL_CHECK_STATUS(aksl_malloc(SIZE_MAX / 2, &ptr), ENOMEM); + /* Never garbage on the failure path. */ + AKSL_CHECK(ptr == NULL); + return 0; +} + +static int test_calloc_zeroes_and_guards(void) +{ + unsigned char *buf = NULL; + void *ptr = (void *)0x1; + size_t i = 0; + + AKSL_CHECK_OK(aksl_calloc(16, sizeof(unsigned char), (void **)&buf)); + AKSL_CHECK(buf != NULL); + for ( i = 0; i < 16; i++ ) { + AKSL_CHECK(buf[i] == 0); + } + AKSL_CHECK_OK(aksl_free(buf)); + + AKSL_CHECK_STATUS(aksl_calloc(16, 1, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_calloc(0, 1, &ptr), AKERR_VALUE); + AKSL_CHECK_STATUS(aksl_calloc(1, 0, &ptr), AKERR_VALUE); + return 0; +} + +/* + * realloc(3)'s trap: on failure it returns NULL and the original block is *still + * valid*, so `p = realloc(p, n)` leaks it. The wrapper takes the pointer by + * reference and leaves it untouched on failure, so the caller still holds a + * pointer it can free -- which is what this asserts by freeing it. + */ +static int test_realloc_grows_and_preserves_the_old_block_on_failure(void) +{ + unsigned char *buf = NULL; + void *ptr = NULL; + size_t i = 0; + + AKSL_CHECK_OK(aksl_malloc(8, (void **)&buf)); + for ( i = 0; i < 8; i++ ) { + buf[i] = (unsigned char)i; + } + + ptr = buf; + AKSL_CHECK_OK(aksl_realloc(&ptr, 64)); + buf = (unsigned char *)ptr; + AKSL_CHECK(buf != NULL); + /* The contents that fit are carried over. */ + for ( i = 0; i < 8; i++ ) { + AKSL_CHECK(buf[i] == (unsigned char)i); + } + + /* A request that cannot be met leaves ptr aimed at the original block. */ + AKSL_CHECK_STATUS(aksl_realloc(&ptr, SIZE_MAX / 2), ENOMEM); + AKSL_CHECK(ptr == (void *)buf); + for ( i = 0; i < 8; i++ ) { + AKSL_CHECK(buf[i] == (unsigned char)i); + } + + AKSL_CHECK_OK(aksl_free(ptr)); + + AKSL_CHECK_STATUS(aksl_realloc(NULL, 8), AKERR_NULLPOINTER); return 0; } static int test_free_rejects_null_pointer(void) { AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_free(NULL), - AKERR_NULLPOINTER, "NULL"); + AKERR_NULLPOINTER, "NULL"); + return 0; +} + +/* aksl_freep clears the caller's pointer, so a second free is caught. */ +static int test_freep_clears_the_pointer(void) +{ + void *ptr = NULL; + + AKSL_CHECK_OK(aksl_malloc(16, &ptr)); + AKSL_CHECK(ptr != NULL); + AKSL_CHECK_OK(aksl_freep(&ptr)); + AKSL_CHECK(ptr == NULL); + /* The second attempt is now an error instead of a double free. */ + AKSL_CHECK_STATUS(aksl_freep(&ptr), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_freep(NULL), AKERR_NULLPOINTER); return 0; } @@ -79,7 +188,7 @@ static int test_memset_zero_length_is_noop(void) static int test_memset_rejects_null_buffer(void) { AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_memset(NULL, 0x00, 4), - AKERR_NULLPOINTER, "s="); + AKERR_NULLPOINTER, "s="); return 0; } @@ -108,12 +217,45 @@ static int test_memcpy_zero_length_is_noop(void) return 0; } +/* + * TODO.md 1.1's open question, decided: overlap is AKERR_VALUE. memcpy(3) calls + * it undefined behaviour, which in practice means "works until the day a + * compiler version or a length changes and it does not". Callers who mean to + * overlap want aksl_memmove, and the message says so. + */ +static int test_memcpy_rejects_overlapping_ranges(void) +{ + unsigned char buf[8] = { 0, 1, 2, 3, 4, 5, 6, 7 }; + + /* Destination inside the source range. */ + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_memcpy(&buf[2], &buf[0], 4), + AKERR_VALUE, "aksl_memmove"); + /* Source inside the destination range. */ + AKSL_CHECK_STATUS(aksl_memcpy(&buf[0], &buf[2], 4), AKERR_VALUE); + /* Adjacent but not overlapping is fine. */ + AKSL_CHECK_OK(aksl_memcpy(&buf[4], &buf[0], 4)); + AKSL_CHECK(buf[4] == 0 && buf[5] == 1 && buf[6] == 2 && buf[7] == 3); + return 0; +} + +static int test_memmove_handles_overlap(void) +{ + unsigned char buf[8] = { 0, 1, 2, 3, 4, 5, 6, 7 }; + + AKSL_CHECK_OK(aksl_memmove(&buf[2], &buf[0], 4)); + AKSL_CHECK(buf[2] == 0 && buf[3] == 1 && buf[4] == 2 && buf[5] == 3); + + AKSL_CHECK_STATUS(aksl_memmove(NULL, buf, 1), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_memmove(buf, NULL, 1), AKERR_NULLPOINTER); + return 0; +} + static int test_memcpy_rejects_null_destination(void) { unsigned char src[1] = { 1 }; AKSL_CHECK_STATUS(aksl_memcpy(NULL, src, sizeof(src)), - AKERR_NULLPOINTER); + AKERR_NULLPOINTER); return 0; } @@ -122,7 +264,50 @@ static int test_memcpy_rejects_null_source(void) unsigned char dst[1] = { 0 }; AKSL_CHECK_STATUS(aksl_memcpy(dst, NULL, sizeof(dst)), - AKERR_NULLPOINTER); + AKERR_NULLPOINTER); + return 0; +} + +/* + * memcmp and memchr answer through an out-param, because the return value is + * already spoken for by the error context. A memchr that finds nothing is not an + * error: "absent" is an ordinary answer to that question, so *dest is NULL and + * the call succeeds. + */ +static int test_memcmp_reports_ordering(void) +{ + unsigned char a[4] = { 1, 2, 3, 4 }; + unsigned char b[4] = { 1, 2, 3, 4 }; + unsigned char c[4] = { 1, 2, 9, 4 }; + int result = 99; + + AKSL_CHECK_OK(aksl_memcmp(a, b, sizeof(a), &result)); + AKSL_CHECK(result == 0); + AKSL_CHECK_OK(aksl_memcmp(a, c, sizeof(a), &result)); + AKSL_CHECK(result < 0); + AKSL_CHECK_OK(aksl_memcmp(c, a, sizeof(a), &result)); + AKSL_CHECK(result > 0); + + AKSL_CHECK_STATUS(aksl_memcmp(NULL, b, 4, &result), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_memcmp(a, NULL, 4, &result), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_memcmp(a, b, 4, NULL), AKERR_NULLPOINTER); + return 0; +} + +static int test_memchr_finds_or_reports_absence(void) +{ + unsigned char buf[5] = { 'a', 'b', 'c', 'd', 'e' }; + void *found = NULL; + + AKSL_CHECK_OK(aksl_memchr(buf, 'c', sizeof(buf), &found)); + AKSL_CHECK(found == (void *)&buf[2]); + + /* Not found is a successful answer of "nowhere". */ + AKSL_CHECK_OK(aksl_memchr(buf, 'z', sizeof(buf), &found)); + AKSL_CHECK(found == NULL); + + AKSL_CHECK_STATUS(aksl_memchr(NULL, 'a', 1, &found), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_memchr(buf, 'a', 1, NULL), AKERR_NULLPOINTER); return 0; } @@ -134,17 +319,29 @@ int main(void) AKSL_RUN(failures, test_malloc_writes_nonnull_pointer); AKSL_RUN(failures, test_malloc_rejects_null_destination); - AKSL_RUN(failures, test_free_rejects_null_pointer); + AKSL_RUN(failures, test_malloc_rejects_zero_size); + AKSL_RUN(failures, test_malloc_reports_out_of_memory); AKSL_RUN(failures, test_malloc_free_round_trip); + AKSL_RUN(failures, test_calloc_zeroes_and_guards); + AKSL_RUN(failures, test_realloc_grows_and_preserves_the_old_block_on_failure); + + AKSL_RUN(failures, test_free_rejects_null_pointer); + AKSL_RUN(failures, test_freep_clears_the_pointer); + AKSL_RUN(failures, test_memset_fills_buffer); AKSL_RUN(failures, test_memset_zero_length_is_noop); AKSL_RUN(failures, test_memset_rejects_null_buffer); AKSL_RUN(failures, test_memcpy_copies_bytes); AKSL_RUN(failures, test_memcpy_zero_length_is_noop); + AKSL_RUN(failures, test_memcpy_rejects_overlapping_ranges); AKSL_RUN(failures, test_memcpy_rejects_null_destination); AKSL_RUN(failures, test_memcpy_rejects_null_source); + AKSL_RUN(failures, test_memmove_handles_overlap); + + AKSL_RUN(failures, test_memcmp_reports_ordering); + AKSL_RUN(failures, test_memchr_finds_or_reports_absence); AKSL_REPORT(failures); } diff --git a/tests/test_path.c b/tests/test_path.c index a8a359a..b870d1e 100644 --- a/tests/test_path.c +++ b/tests/test_path.c @@ -1,19 +1,17 @@ /* - * aksl_realpath -- TODO.md section 1.5. + * aksl_realpath and aksl_realpath_alloc -- TODO.md section 1.5, now complete. * * The happy paths compare against realpath(3) itself rather than against a * hard-coded string, because $TMPDIR may itself be a symlink (/tmp -> /private/tmp * and friends) and the resolved answer is what the platform says it is. * - * Every failure case here passes a *zeroed* resolved_path buffer. That is - * deliberate: on failure the wrapper formats resolved_path with %s while - * realpath(3) leaves the buffer unspecified (TODO.md 2.1.6), so a test that - * passed an uninitialised buffer would be reading uninitialised memory in the - * library's own error path. The uninitialised-buffer crash is the defect's own - * test to write, not something these should trip over incidentally. - * - * Also not covered: resolved_path == NULL, which is unchecked today and leaks - * the buffer realpath(3) allocates (2.1.6). + * The failure cases now pass an *uninitialised* resolved_path on purpose. That + * used to be the crash case (TODO.md 2.1.6): the wrapper's own error path + * formatted the buffer with %s while realpath(3) leaves its contents + * unspecified on failure, so the library read uninitialised memory while + * reporting an error. The message names only the input path now, and this test + * is what holds that -- it is meant to be run under the sanitizer build, where a + * regression is an immediate abort rather than a silent read of stack garbage. */ #include "aksl_capture.h" @@ -31,7 +29,7 @@ static int test_resolves_an_existing_file(void) memset(resolved, 0x00, sizeof(resolved)); AKSL_CHECK(realpath(path, expected) != NULL); - AKSL_CHECK_OK(aksl_realpath(path, resolved)); + AKSL_CHECK_OK(aksl_realpath(path, resolved, sizeof(resolved))); AKSL_CHECK(strcmp(resolved, expected) == 0); AKSL_CHECK(resolved[0] == '/'); AKSL_CHECK(unlink(path) == 0); @@ -54,7 +52,7 @@ static int test_resolves_a_symlink_to_its_target(void) memset(resolved, 0x00, sizeof(resolved)); AKSL_CHECK(realpath(target, expected) != NULL); - AKSL_CHECK_OK(aksl_realpath(link, resolved)); + AKSL_CHECK_OK(aksl_realpath(link, resolved, sizeof(resolved))); AKSL_CHECK(strcmp(resolved, expected) == 0); AKSL_CHECK(unlink(link) == 0); @@ -62,14 +60,20 @@ static int test_resolves_a_symlink_to_its_target(void) return 0; } +/* + * The failure path with a buffer nobody has written to. Uninitialised on + * purpose: see the header comment. Under ASan/MSan this is the test that fails + * if the error path ever starts reading resolved_path again. + */ static int test_missing_path_reports_enoent(void) { char resolved[PATH_MAX]; - memset(resolved, 0x00, sizeof(resolved)); AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_realpath("/nonexistent/aksl/path", resolved), + aksl_realpath("/nonexistent/aksl/path", resolved, sizeof(resolved)), ENOENT, "/nonexistent/aksl/path"); + /* The message must name the path and must not quote the buffer back. */ + AKSL_CHECK(strstr(aksl_last_message, "resolved") == NULL); return 0; } @@ -84,19 +88,94 @@ static int test_non_directory_component_reports_enotdir(void) AKSL_CHECK((size_t)snprintf(child, sizeof(child), "%s/child", path) < sizeof(child)); - memset(resolved, 0x00, sizeof(resolved)); - AKSL_CHECK_STATUS(aksl_realpath(child, resolved), ENOTDIR); + AKSL_CHECK_STATUS(aksl_realpath(child, resolved, sizeof(resolved)), ENOTDIR); AKSL_CHECK(unlink(path) == 0); return 0; } -static int test_rejects_null_path(void) +/* Two symlinks pointing at each other: the kernel gives up with ELOOP. */ +static int test_symlink_loop_reports_eloop(void) +{ + char a[AKSL_TMP_MAX]; + char b[AKSL_TMP_MAX]; + char resolved[PATH_MAX]; + + AKSL_CHECK(aksl_temp_file(a, sizeof(a)) == 0); + AKSL_CHECK(aksl_temp_file(b, sizeof(b)) == 0); + AKSL_CHECK(unlink(a) == 0); + AKSL_CHECK(unlink(b) == 0); + AKSL_CHECK(symlink(a, b) == 0); + AKSL_CHECK(symlink(b, a) == 0); + + AKSL_CHECK_STATUS(aksl_realpath(a, resolved, sizeof(resolved)), ELOOP); + + AKSL_CHECK(unlink(a) == 0); + AKSL_CHECK(unlink(b) == 0); + return 0; +} + +static int test_rejects_null_arguments(void) { char resolved[PATH_MAX]; memset(resolved, 0x00, sizeof(resolved)); - AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_realpath(NULL, resolved), - AKERR_NULLPOINTER, "path="); + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_realpath(NULL, resolved, sizeof(resolved)), + AKERR_NULLPOINTER, "path="); + /* + * TODO.md 2.1.6: this used to be unchecked, and realpath(path, NULL) + * allocated a buffer that the wrapper then discarded and leaked. + */ + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_realpath("/tmp", NULL, PATH_MAX), + AKERR_NULLPOINTER, "resolved_path="); + return 0; +} + +/* + * realpath(3) cannot be told how much room it has, so the only safe answer to an + * undersized buffer is to refuse before calling it. A caller who gets this back + * has a bug that would otherwise have been a stack smash. + */ +static int test_rejects_a_buffer_below_path_max(void) +{ + char small[16]; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_realpath("/tmp", small, sizeof(small)), + AKERR_OUTOFBOUNDS, "PATH_MAX"); + return 0; +} + +static int test_alloc_form_resolves_and_hands_over_the_buffer(void) +{ + char path[AKSL_TMP_MAX]; + char expected[PATH_MAX]; + char *resolved = NULL; + + AKSL_CHECK(aksl_temp_file(path, sizeof(path)) == 0); + AKSL_CHECK(realpath(path, expected) != NULL); + + AKSL_CHECK_OK(aksl_realpath_alloc(path, &resolved)); + AKSL_CHECK(resolved != NULL); + AKSL_CHECK(strcmp(resolved, expected) == 0); + /* The buffer is the caller's; releasing it through the library closes the + * leak that the old NULL-destination path opened. */ + AKSL_CHECK_OK(aksl_free(resolved)); + + AKSL_CHECK(unlink(path) == 0); + return 0; +} + +static int test_alloc_form_reports_failure_and_writes_no_pointer(void) +{ + char *resolved = (char *)0x1; + + AKSL_CHECK_STATUS_MSG_CONTAINS( + aksl_realpath_alloc("/nonexistent/aksl/path", &resolved), + ENOENT, "/nonexistent/aksl/path"); + /* Cleared before the call, so a failure cannot leave a stale pointer. */ + AKSL_CHECK(resolved == NULL); + + AKSL_CHECK_STATUS(aksl_realpath_alloc(NULL, &resolved), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_realpath_alloc("/tmp", NULL), AKERR_NULLPOINTER); return 0; } @@ -110,7 +189,11 @@ int main(void) AKSL_RUN(failures, test_resolves_a_symlink_to_its_target); AKSL_RUN(failures, test_missing_path_reports_enoent); AKSL_RUN(failures, test_non_directory_component_reports_enotdir); - AKSL_RUN(failures, test_rejects_null_path); + AKSL_RUN(failures, test_symlink_loop_reports_eloop); + AKSL_RUN(failures, test_rejects_null_arguments); + AKSL_RUN(failures, test_rejects_a_buffer_below_path_max); + AKSL_RUN(failures, test_alloc_form_resolves_and_hands_over_the_buffer); + AKSL_RUN(failures, test_alloc_form_reports_failure_and_writes_no_pointer); AKSL_REPORT(failures); } diff --git a/tests/test_stream.c b/tests/test_stream.c index b0b2a0f..5a9833f 100644 --- a/tests/test_stream.c +++ b/tests/test_stream.c @@ -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 +#include 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); diff --git a/tests/test_strhash.c b/tests/test_strhash.c index 57fd3f2..fb90db1 100644 --- a/tests/test_strhash.c +++ b/tests/test_strhash.c @@ -5,9 +5,9 @@ * h = h * 33 + byte for each of len bytes, truncated to 32 bits. They were * computed independently of this implementation. * - * Every vector here is 7-bit ASCII, where signed and unsigned char agree. The - * high-bit case ("\xff\xfe") is the sign-extension defect in TODO.md 2.2.6 and - * is left for a test that can be registered as a known failure. + * Most vectors here are 7-bit ASCII, where signed and unsigned char agree and + * the test therefore says nothing either way about byte signedness. The high-bit + * vector is the one that pins TODO.md 2.2.6 down. */ #include "aksl_capture.h" @@ -46,6 +46,49 @@ static int test_known_answer_vectors(void) return 0; } +/* + * TODO.md 2.2.6, pinned. The cursor is an unsigned char * now, so bytes at or + * above 0x80 contribute their unsigned value. Iterating a plain char * on x86 or + * ARM Linux made them negative, giving 5859874 here instead of 5868578 -- a hash + * that disagreed with canonical djb2 and, worse, disagreed with itself across + * platforms depending on whether char happened to be signed. + * + * Benign for the 7-bit identifiers akbasic hashes; quietly wrong for the first + * caller to key a table on a filename or a UTF-8 string literal. + */ +static int test_high_bit_bytes_are_unsigned(void) +{ + char buf[2] = { (char)0xff, (char)0xfe }; + uint32_t h = 0; + + AKSL_CHECK_OK(aksl_strhash_djb2(buf, sizeof(buf), &h)); + AKSL_CHECK(h == 5868578u); + /* The sign-extended answer, spelled out so a regression names itself. */ + AKSL_CHECK(h != 5859874u); + return 0; +} + +/* The NUL-terminated convenience form (TODO.md 3.6) agrees with the length one. */ +static int test_str_form_matches_the_length_form(void) +{ + const char *s = "libakstdlib"; + uint32_t from_str = 0; + uint32_t from_len = 0; + + AKSL_CHECK_OK(aksl_strhash_djb2_str(s, &from_str)); + AKSL_CHECK_OK(aksl_strhash_djb2(s, strlen(s), &from_len)); + AKSL_CHECK(from_str == from_len); + AKSL_CHECK(from_str == 884285482u); + + /* The empty string is the seed, not an error. */ + AKSL_CHECK_OK(aksl_strhash_djb2_str("", &from_str)); + AKSL_CHECK(from_str == 5381); + + AKSL_CHECK_STATUS(aksl_strhash_djb2_str(NULL, &from_str), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strhash_djb2_str("x", NULL), AKERR_NULLPOINTER); + return 0; +} + /* The function is length-driven, not NUL-driven: an embedded NUL is hashed. */ static int test_embedded_nul_is_hashed(void) { @@ -95,6 +138,8 @@ int main(void) AKSL_RUN(failures, test_empty_string_is_the_djb2_seed); AKSL_RUN(failures, test_zero_length_ignores_the_buffer); AKSL_RUN(failures, test_known_answer_vectors); + AKSL_RUN(failures, test_high_bit_bytes_are_unsigned); + AKSL_RUN(failures, test_str_form_matches_the_length_form); AKSL_RUN(failures, test_embedded_nul_is_hashed); AKSL_RUN(failures, test_hash_is_stable_across_calls); AKSL_RUN(failures, test_rejects_null_arguments); diff --git a/tests/test_strto.c b/tests/test_strto.c new file mode 100644 index 0000000..031b354 --- /dev/null +++ b/tests/test_strto.c @@ -0,0 +1,292 @@ +/* + * The strto* family -- TODO.md section 3.1. + * + * These are the real implementation behind the ato* wrappers and the thing + * akbasic had to hand-write for itself (its src/convert.c, ~60 lines) because + * the library would not do it. What they add over the ato* forms is a base and + * an endptr: pass a non-NULL endptr to parse a number off the front of a longer + * string and find out where it stopped, and the trailing-junk check goes away + * because the caller has taken responsibility for what follows. + * + * tests/test_convert.c covers what the two families share; this file is the part + * that is specific to having a base and an endptr. + */ + +#include "aksl_capture.h" + +#include +#include + +/* ---------------------------------------------------------------------- */ +/* Base handling */ +/* ---------------------------------------------------------------------- */ + +static int test_base_is_honoured(void) +{ + long out = 0; + + AKSL_CHECK_OK(aksl_strtol("ff", NULL, 16, &out)); + AKSL_CHECK(out == 255); + AKSL_CHECK_OK(aksl_strtol("1010", NULL, 2, &out)); + AKSL_CHECK(out == 10); + AKSL_CHECK_OK(aksl_strtol("777", NULL, 8, &out)); + AKSL_CHECK(out == 511); + AKSL_CHECK_OK(aksl_strtol("z", NULL, 36, &out)); + AKSL_CHECK(out == 35); + return 0; +} + +/* Base 0 auto-detects the 0x and 0 prefixes, which is the counterpart to + * tests/test_convert.c's test that aksl_atoi refuses "0x10". */ +static int test_base_zero_detects_the_prefix(void) +{ + long out = 0; + + AKSL_CHECK_OK(aksl_strtol("0x10", NULL, 0, &out)); + AKSL_CHECK(out == 16); + AKSL_CHECK_OK(aksl_strtol("010", NULL, 0, &out)); + AKSL_CHECK(out == 8); + AKSL_CHECK_OK(aksl_strtol("10", NULL, 0, &out)); + AKSL_CHECK(out == 10); + return 0; +} + +/* A digit that does not belong to the base is where the number ends. */ +static int test_digits_outside_the_base_end_the_number(void) +{ + long out = 0; + char *end = NULL; + + /* "9" is not an octal digit, so with a NULL endptr it is trailing junk. */ + AKSL_CHECK_STATUS(aksl_strtol("129", NULL, 8, &out), AKERR_VALUE); + /* With an endptr the caller gets the partial parse and the stopping point. */ + AKSL_CHECK_OK(aksl_strtol("129", &end, 8, &out)); + AKSL_CHECK(out == 10); /* 012 octal */ + AKSL_CHECK(end != NULL && *end == '9'); + return 0; +} + +/* An invalid base is EINVAL from strtol(3) rather than a silent answer. */ +static int test_invalid_base_is_rejected(void) +{ + long out = 99; + + AKSL_CHECK_STATUS(aksl_strtol("10", NULL, 37, &out), EINVAL); + AKSL_CHECK(out == 0); + AKSL_CHECK_STATUS(aksl_strtol("10", NULL, 1, &out), EINVAL); + AKSL_CHECK(out == 0); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* endptr */ +/* ---------------------------------------------------------------------- */ + +/* + * With an endptr, trailing text is the caller's business and not an error. This + * is the shape a tokenizer wants: parse a number, carry on from where it ended. + */ +static int test_endptr_reports_where_parsing_stopped(void) +{ + const char *input = "42 rest of the line"; + char *end = NULL; + long out = 0; + + AKSL_CHECK_OK(aksl_strtol(input, &end, 10, &out)); + AKSL_CHECK(out == 42); + AKSL_CHECK(end == input + 2); + AKSL_CHECK(strcmp(end, " rest of the line") == 0); + return 0; +} + +/* Consuming nothing is still an error, endptr or not: there was no number. */ +static int test_no_digits_is_an_error_even_with_an_endptr(void) +{ + char *end = (char *)0x1; + long out = 99; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_strtol("nope", &end, 10, &out), + AKERR_VALUE, "no digits"); + AKSL_CHECK(out == 0); + return 0; +} + +/* Successive calls chained through endptr walk a whole list of numbers. */ +static int test_endptr_chains_across_a_list(void) +{ + const char *input = "1,2,3"; + char *cursor = (char *)input; + long out = 0; + long total = 0; + int seen = 0; + + while ( *cursor != '\0' ) { + AKSL_CHECK_OK(aksl_strtol(cursor, &cursor, 10, &out)); + total += out; + seen += 1; + if ( *cursor == ',' ) { + cursor++; + } + } + AKSL_CHECK(seen == 3); + AKSL_CHECK(total == 6); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Ranges and signs */ +/* ---------------------------------------------------------------------- */ + +static int test_range_errors_per_type(void) +{ + long lout = 99; + long long llout = 99; + unsigned long ulout = 99; + unsigned long long ullout = 99; + char buf[64]; + + snprintf(buf, sizeof(buf), "%lld9", (long long)LLONG_MAX); + AKSL_CHECK_STATUS(aksl_strtol(buf, NULL, 10, &lout), ERANGE); + AKSL_CHECK(lout == 0); + AKSL_CHECK_STATUS(aksl_strtoll(buf, NULL, 10, &llout), ERANGE); + AKSL_CHECK(llout == 0); + + snprintf(buf, sizeof(buf), "%llu9", (unsigned long long)ULLONG_MAX); + AKSL_CHECK_STATUS(aksl_strtoul(buf, NULL, 10, &ulout), ERANGE); + AKSL_CHECK(ulout == 0); + AKSL_CHECK_STATUS(aksl_strtoull(buf, NULL, 10, &ullout), ERANGE); + AKSL_CHECK(ullout == 0); + return 0; +} + +/* + * strtoul(3) and strtoull(3) accept a leading '-' and hand back the negation + * wrapped into the unsigned range, so "-1" parses as ULONG_MAX and reports + * nothing at all. That is exactly the silent failure this library exists to + * surface, so the sign is refused before the call. + */ +static int test_unsigned_forms_refuse_a_negative(void) +{ + unsigned long ulout = 99; + unsigned long long ullout = 99; + + AKSL_CHECK_STATUS_MSG_CONTAINS(aksl_strtoul("-1", NULL, 10, &ulout), + AKERR_VALUE, "negative"); + AKSL_CHECK(ulout == 0); + /* Including behind leading whitespace, which strtoul skips. */ + AKSL_CHECK_STATUS(aksl_strtoul(" -1", NULL, 10, &ulout), AKERR_VALUE); + AKSL_CHECK_STATUS(aksl_strtoull("-1", NULL, 10, &ullout), AKERR_VALUE); + AKSL_CHECK(ullout == 0); + + /* A leading '+' is fine: it is a sign, not a negation. */ + AKSL_CHECK_OK(aksl_strtoul("+7", NULL, 10, &ulout)); + AKSL_CHECK(ulout == 7); + return 0; +} + +static int test_unsigned_boundaries_round_trip(void) +{ + unsigned long ulout = 0; + unsigned long long ullout = 0; + char buf[64]; + + snprintf(buf, sizeof(buf), "%lu", ULONG_MAX); + AKSL_CHECK_OK(aksl_strtoul(buf, NULL, 10, &ulout)); + AKSL_CHECK(ulout == ULONG_MAX); + + snprintf(buf, sizeof(buf), "%llu", ULLONG_MAX); + AKSL_CHECK_OK(aksl_strtoull(buf, NULL, 10, &ullout)); + AKSL_CHECK(ullout == ULLONG_MAX); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Floating point */ +/* ---------------------------------------------------------------------- */ + +static int test_floating_point_forms(void) +{ + double dout = 0.0; + float fout = 0.0f; + long double ldout = 0.0L; + char *end = NULL; + + AKSL_CHECK_OK(aksl_strtod("2.5", NULL, &dout)); + AKSL_CHECK(dout == 2.5); + AKSL_CHECK_OK(aksl_strtof("2.5", NULL, &fout)); + AKSL_CHECK(fout == 2.5f); + AKSL_CHECK_OK(aksl_strtold("2.5", NULL, &ldout)); + AKSL_CHECK(ldout == 2.5L); + + AKSL_CHECK_OK(aksl_strtod("2.5rest", &end, &dout)); + AKSL_CHECK(dout == 2.5); + AKSL_CHECK(strcmp(end, "rest") == 0); + + AKSL_CHECK_STATUS(aksl_strtod("2.5rest", NULL, &dout), AKERR_VALUE); + AKSL_CHECK_STATUS(aksl_strtof("nope", NULL, &fout), AKERR_VALUE); + AKSL_CHECK_STATUS(aksl_strtold("nope", NULL, &ldout), AKERR_VALUE); + + /* float has a much smaller range than double, and says so. */ + AKSL_CHECK_STATUS(aksl_strtof("1e300", NULL, &fout), ERANGE); + AKSL_CHECK(fout == 0.0f); + AKSL_CHECK_OK(aksl_strtod("1e300", NULL, &dout)); + AKSL_CHECK(dout > 0.0); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Argument validation */ +/* ---------------------------------------------------------------------- */ + +static int test_every_form_rejects_null_arguments(void) +{ + long lout = 0; + long long llout = 0; + unsigned long ulout = 0; + unsigned long long ullout = 0; + double dout = 0.0; + float fout = 0.0f; + long double ldout = 0.0L; + + AKSL_CHECK_STATUS(aksl_strtol(NULL, NULL, 10, &lout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtol("1", NULL, 10, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoll(NULL, NULL, 10, &llout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoll("1", NULL, 10, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoul(NULL, NULL, 10, &ulout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoul("1", NULL, 10, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoull(NULL, NULL, 10, &ullout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtoull("1", NULL, 10, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtod(NULL, NULL, &dout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtod("1", NULL, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtof(NULL, NULL, &fout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtof("1", NULL, NULL), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtold(NULL, NULL, &ldout), AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_strtold("1", NULL, NULL), AKERR_NULLPOINTER); + return 0; +} + +int main(void) +{ + int failures = 0; + + akerr_init(); + + AKSL_RUN(failures, test_base_is_honoured); + AKSL_RUN(failures, test_base_zero_detects_the_prefix); + AKSL_RUN(failures, test_digits_outside_the_base_end_the_number); + AKSL_RUN(failures, test_invalid_base_is_rejected); + + AKSL_RUN(failures, test_endptr_reports_where_parsing_stopped); + AKSL_RUN(failures, test_no_digits_is_an_error_even_with_an_endptr); + AKSL_RUN(failures, test_endptr_chains_across_a_list); + + AKSL_RUN(failures, test_range_errors_per_type); + AKSL_RUN(failures, test_unsigned_forms_refuse_a_negative); + AKSL_RUN(failures, test_unsigned_boundaries_round_trip); + + AKSL_RUN(failures, test_floating_point_forms); + + AKSL_RUN(failures, test_every_form_rejects_null_arguments); + + AKSL_REPORT(failures); +} diff --git a/tests/test_tree.c b/tests/test_tree.c index 201703f..0210c1e 100644 --- a/tests/test_tree.c +++ b/tests/test_tree.c @@ -1,165 +1,572 @@ /* - * Depth-first tree search. + * Tree traversal -- TODO.md section 1.8, complete. * - * Previously this test shared one TreeSearchParams across all three searches - * without resetting it, so `steps` accumulated (7, then 14, then 21) and the - * second assertion failed -- `tree` was a red test on every run. Each search - * now gets its own params. + * The old version of this file counted steps, which cannot tell the three + * depth-first orders apart because all three visit all seven nodes -- and could + * not prove that AKERR_ITERATOR_BREAK stopped anything either, because it hid + * its target in tree[6], the last node visited in every depth-first order. Every + * traversal here records the node pointers it was handed, in order, and compares + * against the expected sequence. * - * Caveat worth knowing when reading the step counts below: the value is hidden - * in tree[6], which is the last node visited in pre-, in- and post-order alike, - * so "7 steps" holds for all three orders and does not actually distinguish - * them -- nor does it prove that AKERR_ITERATOR_BREAK stopped anything (it does - * not; see tests/test_tree_iterate_break.c and TODO.md 2.1.3). Visit-order - * assertions that tell the three traversals apart are TODO.md section 1.8. - */ - -#include "aksl_capture.h" - -#define MAX_LEAVES 7 -#define HIDDEN_VALUE ((void *)17336) - -typedef struct TreeSearchParams -{ - void *value; - int steps; - aksl_TreeNode *node; -} TreeSearchParams; - -static akerr_ErrorContext AKERR_NOIGNORE *find_value(aksl_TreeNode *node, void *data) -{ - TreeSearchParams *parms = NULL; - - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); - FAIL_ZERO_RETURN(e, data, AKERR_NULLPOINTER, "data"); - parms = (TreeSearchParams *)data; - parms->steps += 1; - if ( node->leaf == parms->value ) { - parms->node = node; - FAIL_RETURN(e, AKERR_ITERATOR_BREAK, "stop"); - } - SUCCEED_RETURN(e); -} - -/* - * Build the 3-level tree used by every case here, with the search value hidden - * in the bottom-right leaf. + * The 7-node tree used throughout: * - * LEFT RIGHT * TREE[0] * +--------^^---------+ * | | * TREE[1] TREE[2] * +---^^---+ +---^^---+ * | | | | - *TREE[3] TREE[4] TREE[5] TREE[6] + * TREE[3] TREE[4] TREE[5] TREE[6] + * + * pre-order 0 1 3 4 2 5 6 + * in-order 3 1 4 0 5 2 6 + * post-order 3 4 1 5 6 2 0 + * BFS 0 1 2 3 4 5 6 + * BFS_RIGHT 0 2 1 6 5 4 3 */ -static void build_tree(aksl_TreeNode *tree, TreeSearchParams *parms) + +#include "aksl_capture.h" + +#define MAX_LEAVES 7 +#define MAX_VISITS 32 + +typedef struct VisitLog { - memset((void *)tree, 0x00, sizeof(aksl_TreeNode) * MAX_LEAVES); + int count; + aksl_TreeNode *seen[MAX_VISITS]; + aksl_TreeNode *break_at; /* node to raise ITERATOR_BREAK on, or NULL */ + aksl_TreeNode *fail_at; /* node to raise AKERR_VALUE on, or NULL */ +} VisitLog; + +static void visitlog_init(VisitLog *log) +{ + memset((void *)log, 0x00, sizeof(VisitLog)); +} + +static akerr_ErrorContext AKERR_NOIGNORE *record_visit(aksl_TreeNode *node, void *data) +{ + VisitLog *log = NULL; + + PREPARE_ERROR(e); + FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); + FAIL_ZERO_RETURN(e, data, AKERR_NULLPOINTER, "data"); + log = (VisitLog *)data; + if ( log->count < MAX_VISITS ) { + log->seen[log->count] = node; + } + log->count += 1; + if ( log->fail_at == node ) { + FAIL_RETURN(e, AKERR_VALUE, "iterator failed at node %p", (void *)node); + } + if ( log->break_at == node ) { + FAIL_RETURN(e, AKERR_ITERATOR_BREAK, "stop at node %p", (void *)node); + } + SUCCEED_RETURN(e); +} + +static void build_tree(aksl_TreeNode *tree) +{ + int i = 0; + + for ( i = 0; i < MAX_LEAVES; i++ ) { + memset((void *)&tree[i], 0x00, sizeof(aksl_TreeNode)); + } tree[0].left = &tree[1]; tree[0].right = &tree[2]; tree[1].left = &tree[3]; tree[1].right = &tree[4]; tree[2].left = &tree[5]; tree[2].right = &tree[6]; - tree[6].leaf = HIDDEN_VALUE; - - memset((void *)parms, 0x00, sizeof(TreeSearchParams)); - parms->value = HIDDEN_VALUE; } -static int search_finds_hidden_value(uint8_t searchmode) +/* Assert that the visit log matches `expected`, which is a list of tree indices. */ +static int check_order(VisitLog *log, aksl_TreeNode *tree, + const int *expected, int n) { - aksl_TreeNode tree[MAX_LEAVES]; - TreeSearchParams parms; + int i = 0; - build_tree(tree, &parms); - - AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &find_value, NULL, NULL, - searchmode, &parms, NULL)); - AKSL_CHECK(parms.node == &tree[6]); - AKSL_CHECK(parms.steps == MAX_LEAVES); + if ( log->count != n ) { + fprintf(stderr, " CHECK FAILED: visited %d nodes, expected %d\n", + log->count, n); + return 1; + } + for ( i = 0; i < n; i++ ) { + if ( log->seen[i] != &tree[expected[i]] ) { + fprintf(stderr, " CHECK FAILED: visit %d was node %ld, expected %d\n", + i, (long)(log->seen[i] - tree), expected[i]); + return 1; + } + } return 0; } -static int test_dfs_preorder(void) +/* ---------------------------------------------------------------------- */ +/* aksl_tree_node_init */ +/* ---------------------------------------------------------------------- */ + +static int test_node_init_zeroes_the_links(void) { - return search_finds_hidden_value(AKSL_TREE_SEARCH_DFS_PREORDER); + aksl_TreeNode node; + int payload = 3; + + memset((void *)&node, 0xff, sizeof(node)); + AKSL_CHECK_OK(aksl_tree_node_init(&node, &payload)); + AKSL_CHECK(node.parent == NULL); + AKSL_CHECK(node.left == NULL); + AKSL_CHECK(node.right == NULL); + AKSL_CHECK(node.leaf == (void *)&payload); + + AKSL_CHECK_STATUS(aksl_tree_node_init(NULL, NULL), AKERR_NULLPOINTER); + return 0; } -static int test_dfs_inorder(void) +/* ---------------------------------------------------------------------- */ +/* Traversal orders */ +/* ---------------------------------------------------------------------- */ + +static int test_preorder_visits_root_left_right(void) { - return search_finds_hidden_value(AKSL_TREE_SEARCH_DFS_INORDER); + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + static const int expected[MAX_LEAVES] = { 0, 1, 3, 4, 2, 5, 6 }; + + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); + return 0; } -static int test_dfs_postorder(void) +static int test_inorder_visits_left_root_right(void) { - return search_finds_hidden_value(AKSL_TREE_SEARCH_DFS_POSTORDER); + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + static const int expected[MAX_LEAVES] = { 3, 1, 4, 0, 5, 2, 6 }; + + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_INORDER, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); + return 0; +} + +static int test_postorder_visits_left_right_root(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + static const int expected[MAX_LEAVES] = { 3, 4, 1, 5, 6, 2, 0 }; + + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_POSTORDER, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); + return 0; +} + +/* AKSL_TREE_SEARCH_DFS is documented as an alias for the pre-order mode. */ +static int test_dfs_is_an_alias_for_preorder(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + static const int expected[MAX_LEAVES] = { 0, 1, 3, 4, 2, 5, 6 }; + + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); + return 0; } /* - * Both breadth-first modes are declared in the header but not implemented, and - * say so through AKERR_NOT_IMPLEMENTED rather than by silently visiting nothing. - * TODO.md 2.2.10 tracks implementing them; until then this is the contract. + * BFS was AKERR_NOT_IMPLEMENTED, and the lalloc/lfree parameters that existed to + * serve it were defaulted and then never called -- TODO.md 2.2.8 and 2.2.10. + * Both modes work now, and the allocator test below proves the queue is real. */ -static int bfs_reports_not_implemented(uint8_t searchmode) +static int test_bfs_visits_level_by_level(void) { aksl_TreeNode tree[MAX_LEAVES]; - TreeSearchParams parms; + VisitLog log; + static const int expected[MAX_LEAVES] = { 0, 1, 2, 3, 4, 5, 6 }; - build_tree(tree, &parms); - - AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_tree_iterate(&tree[0], &find_value, NULL, NULL, searchmode, - &parms, NULL), - AKERR_NOT_IMPLEMENTED, "Searchmode"); - AKSL_CHECK(parms.steps == 0); - AKSL_CHECK(parms.node == NULL); + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); return 0; } -static int test_bfs_is_not_implemented(void) -{ - return bfs_reports_not_implemented(AKSL_TREE_SEARCH_BFS); -} - -static int test_bfs_right_is_not_implemented(void) -{ - return bfs_reports_not_implemented(AKSL_TREE_SEARCH_BFS_RIGHT); -} - -static int test_iterate_null_arguments(void) +static int test_bfs_right_visits_right_child_first(void) { aksl_TreeNode tree[MAX_LEAVES]; - TreeSearchParams parms; + VisitLog log; + static const int expected[MAX_LEAVES] = { 0, 2, 1, 6, 5, 4, 3 }; - build_tree(tree, &parms); - - AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_tree_iterate(NULL, &find_value, NULL, NULL, - AKSL_TREE_SEARCH_DFS_PREORDER, &parms, NULL), - AKERR_NULLPOINTER, "root"); - AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_tree_iterate(&tree[0], NULL, NULL, NULL, - AKSL_TREE_SEARCH_DFS_PREORDER, &parms, NULL), - AKERR_NULLPOINTER, "iter"); + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS_RIGHT, &log)); + AKSL_CHECK(check_order(&log, tree, expected, MAX_LEAVES) == 0); return 0; } -/* A callback error that is not AKERR_ITERATOR_BREAK reaches the caller. */ -static int test_iterate_propagates_callback_error(void) +/* AKSL_TREE_SEARCH_VISIT: this node and no further. */ +static int test_visit_mode_does_not_descend(void) { aksl_TreeNode tree[MAX_LEAVES]; - TreeSearchParams parms; + VisitLog log; + static const int expected[1] = { 0 }; - build_tree(tree, &parms); - /* find_value raises AKERR_NULLPOINTER when it is handed no data. */ + build_tree(tree); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_VISIT, &log)); + AKSL_CHECK(check_order(&log, tree, expected, 1) == 0); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Custom allocator */ +/* ---------------------------------------------------------------------- */ + +static int counting_alloc_calls = 0; +static int counting_free_calls = 0; + +static akerr_ErrorContext AKERR_NOIGNORE *counting_alloc(size_t size, void **dest) +{ + counting_alloc_calls += 1; + return aksl_malloc(size, dest); +} + +static akerr_ErrorContext AKERR_NOIGNORE *counting_free(void *ptr) +{ + counting_free_calls += 1; + return aksl_free(ptr); +} + +/* + * TODO.md 1.8: "Custom lalloc/lfree are actually invoked -- currently they are + * stored and never called". They are called now, once per node enqueued, and + * every allocation is released. The depth-first modes allocate nothing at all, + * which is the other half of the contract. + */ +static int test_custom_allocator_is_used_and_balanced(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + visitlog_init(&log); + counting_alloc_calls = 0; + counting_free_calls = 0; + + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, + &counting_alloc, &counting_free, + AKSL_TREE_SEARCH_BFS, &log)); + AKSL_CHECK(log.count == MAX_LEAVES); + /* One queue entry per node visited, and every one of them released. */ + AKSL_CHECK(counting_alloc_calls == MAX_LEAVES); + AKSL_CHECK(counting_free_calls == MAX_LEAVES); + + /* Depth-first needs no queue, so it must not touch the allocator. */ + counting_alloc_calls = 0; + counting_free_calls = 0; + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, + &counting_alloc, &counting_free, + AKSL_TREE_SEARCH_DFS_PREORDER, &log)); + AKSL_CHECK(counting_alloc_calls == 0); + AKSL_CHECK(counting_free_calls == 0); + return 0; +} + +/* A break part-way through a BFS still drains the queue it had already built. */ +static int test_break_during_bfs_releases_the_queue(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + visitlog_init(&log); + log.break_at = &tree[1]; + counting_alloc_calls = 0; + counting_free_calls = 0; + + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, + &counting_alloc, &counting_free, + AKSL_TREE_SEARCH_BFS, &log)); + /* 0 and 1 visited; the walk stops at 1 with 2 still sitting in the queue. */ + AKSL_CHECK(log.count == 2); + AKSL_CHECK(counting_alloc_calls > 0); + AKSL_CHECK(counting_alloc_calls == counting_free_calls); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Degenerate shapes */ +/* ---------------------------------------------------------------------- */ + +static int test_single_node_tree_visits_once_in_every_order(void) +{ + aksl_TreeNode node; + VisitLog log; + static const uint8_t modes[] = { + AKSL_TREE_SEARCH_DFS_PREORDER, + AKSL_TREE_SEARCH_DFS_INORDER, + AKSL_TREE_SEARCH_DFS_POSTORDER, + AKSL_TREE_SEARCH_BFS, + AKSL_TREE_SEARCH_BFS_RIGHT, + AKSL_TREE_SEARCH_VISIT, + }; + size_t i = 0; + + for ( i = 0; i < sizeof(modes) / sizeof(modes[0]); i++ ) { + AKSL_CHECK_OK(aksl_tree_node_init(&node, NULL)); + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&node, &record_visit, NULL, NULL, + modes[i], &log)); + AKSL_CHECK(log.count == 1); + AKSL_CHECK(log.seen[0] == &node); + } + return 0; +} + +/* Left-only and right-only chains of three nodes, no branching anywhere. */ +static int test_degenerate_chains(void) +{ + aksl_TreeNode chain[3]; + VisitLog log; + static const int forward[3] = { 0, 1, 2 }; + static const int backward[3] = { 2, 1, 0 }; + int i = 0; + + /* Left-only: 0 -> 1 -> 2 */ + for ( i = 0; i < 3; i++ ) { + AKSL_CHECK_OK(aksl_tree_node_init(&chain[i], NULL)); + } + chain[0].left = &chain[1]; + chain[1].left = &chain[2]; + + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log)); + AKSL_CHECK(check_order(&log, chain, forward, 3) == 0); + + /* In-order down a left chain arrives at the deepest node first. */ + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_INORDER, &log)); + AKSL_CHECK(check_order(&log, chain, backward, 3) == 0); + + /* Right-only: the same shape, mirrored. */ + for ( i = 0; i < 3; i++ ) { + AKSL_CHECK_OK(aksl_tree_node_init(&chain[i], NULL)); + } + chain[0].right = &chain[1]; + chain[1].right = &chain[2]; + + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_INORDER, &log)); + AKSL_CHECK(check_order(&log, chain, forward, 3) == 0); + + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS, &log)); + AKSL_CHECK(check_order(&log, chain, forward, 3) == 0); + return 0; +} + +/* + * TODO.md 1.8 / 2.2.7: a chain deeper than the recursion can take. It used to + * overflow the stack; it is AKERR_OUTOFBOUNDS now, and the message names the + * documented limit. Built one node past the cap so the failure is the cap itself + * and not some incidental shortfall. `static` because AKSL_TREE_MAX_DEPTH nodes + * on the stack of a test function is not the point of the test. + */ +static int test_tree_deeper_than_the_cap_is_out_of_bounds(void) +{ + static aksl_TreeNode chain[AKSL_TREE_MAX_DEPTH + 2]; + VisitLog log; + int i = 0; + + for ( i = 0; i < AKSL_TREE_MAX_DEPTH + 2; i++ ) { + AKSL_CHECK_OK(aksl_tree_node_init(&chain[i], NULL)); + } + for ( i = 0; i < AKSL_TREE_MAX_DEPTH + 1; i++ ) { + chain[i].left = &chain[i + 1]; + } + + visitlog_init(&log); AKSL_CHECK_STATUS_MSG_CONTAINS( - aksl_tree_iterate(&tree[0], &find_value, NULL, NULL, - AKSL_TREE_SEARCH_DFS_PREORDER, NULL, NULL), - AKERR_NULLPOINTER, "data"); + aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log), + AKERR_OUTOFBOUNDS, "AKSL_TREE_MAX_DEPTH"); + + /* Exactly at the cap is fine -- a limit, not an off-by-one. */ + chain[AKSL_TREE_MAX_DEPTH - 1].left = NULL; + visitlog_init(&log); + AKSL_CHECK_OK(aksl_tree_iterate(&chain[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log)); + AKSL_CHECK(log.count == AKSL_TREE_MAX_DEPTH); + return 0; +} + +/* + * A child pointing back at an ancestor. This used to recurse until the process + * died. The depth-first walk carries the ancestor chain and recognises the node + * as its own forebear; the breadth-first walk has no ancestor chain to compare + * against, so the same tree comes back as AKERR_OUTOFBOUNDS through the depth + * cap instead -- a different status for the same shape, which the header says + * out loud rather than leaving to be discovered. + */ +static int test_cyclic_tree_is_caught(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + tree[3].left = &tree[0]; /* back to the root */ + + visitlog_init(&log); + AKSL_CHECK_STATUS_MSG_CONTAINS( + aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log), + AKERR_CIRCULAR_REFERENCE, "own ancestor"); + + visitlog_init(&log); + AKSL_CHECK_STATUS(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS, &log), + AKERR_OUTOFBOUNDS); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Callback control flow */ +/* ---------------------------------------------------------------------- */ + +/* + * The defect that made ITERATOR_BREAK useless on a tree: the recursive frame + * that raised the break handled it in its own PROCESS/HANDLE block and returned + * success, so the parent frame's PASS saw nothing wrong and carried straight on + * into the sibling subtree. All seven nodes were visited no matter where the + * break was raised. TODO.md 2.1.3. + * + * One case per order, each breaking on a node that is *not* last in that order -- + * which is precisely what the old test could not do, because it hid its target + * in tree[6], the final node in all three depth-first walks. + */ +static int test_break_aborts_the_whole_traversal(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + + /* Pre-order 0 1 3 ... : breaking at 3 is the third visit. */ + visitlog_init(&log); + log.break_at = &tree[3]; + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log)); + AKSL_CHECK(log.count == 3); + + /* In-order 3 1 4 ... : breaking at 4 is the third visit. */ + visitlog_init(&log); + log.break_at = &tree[4]; + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_INORDER, &log)); + AKSL_CHECK(log.count == 3); + + /* Post-order 3 4 1 5 ... : breaking at 5 is the fourth visit. */ + visitlog_init(&log); + log.break_at = &tree[5]; + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_POSTORDER, &log)); + AKSL_CHECK(log.count == 4); + + /* BFS 0 1 2 3 ... : breaking at 2 is the third visit. */ + visitlog_init(&log); + log.break_at = &tree[2]; + AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS, &log)); + AKSL_CHECK(log.count == 3); + return 0; +} + +/* Any other status propagates out with its message and its own status intact. */ +static int test_callback_error_propagates(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + visitlog_init(&log); + log.fail_at = &tree[3]; + + AKSL_CHECK_STATUS_MSG_CONTAINS( + aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log), + AKERR_VALUE, "iterator failed at node"); + AKSL_CHECK(log.count == 3); + + /* And out of a BFS, where the queue has to be drained on the way past. */ + visitlog_init(&log); + log.fail_at = &tree[2]; + AKSL_CHECK_STATUS(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_BFS, &log), + AKERR_VALUE); + return 0; +} + +/* ---------------------------------------------------------------------- */ +/* Argument validation */ +/* ---------------------------------------------------------------------- */ + +static int test_null_arguments(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + visitlog_init(&log); + + AKSL_CHECK_STATUS(aksl_tree_iterate(NULL, &record_visit, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log), + AKERR_NULLPOINTER); + AKSL_CHECK_STATUS(aksl_tree_iterate(&tree[0], NULL, NULL, NULL, + AKSL_TREE_SEARCH_DFS_PREORDER, &log), + AKERR_NULLPOINTER); + AKSL_CHECK(log.count == 0); + return 0; +} + +/* + * TODO.md 2.2.9: the switch had no default, so an unrecognised mode -- and + * AKSL_TREE_SEARCH_VISIT, which the header documented but nothing implemented -- + * fell straight through to SUCCEED_RETURN having visited nothing at all. A + * traversal that silently did not happen, reported as success. + */ +static int test_unknown_searchmode_is_a_value_error(void) +{ + aksl_TreeNode tree[MAX_LEAVES]; + VisitLog log; + + build_tree(tree); + visitlog_init(&log); + + AKSL_CHECK_STATUS_MSG_CONTAINS( + aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, 99, &log), + AKERR_VALUE, "unknown searchmode"); + AKSL_CHECK(log.count == 0); + + /* 6 is one past the last defined mode, and just as unacceptable. */ + AKSL_CHECK_STATUS(aksl_tree_iterate(&tree[0], &record_visit, NULL, NULL, 6, &log), + AKERR_VALUE); + AKSL_CHECK(log.count == 0); return 0; } @@ -169,14 +576,29 @@ int main(void) akerr_init(); - AKSL_RUN(failures, test_dfs_preorder); - AKSL_RUN(failures, test_dfs_inorder); - AKSL_RUN(failures, test_dfs_postorder); + AKSL_RUN(failures, test_node_init_zeroes_the_links); - AKSL_RUN(failures, test_bfs_is_not_implemented); - AKSL_RUN(failures, test_bfs_right_is_not_implemented); - AKSL_RUN(failures, test_iterate_null_arguments); - AKSL_RUN(failures, test_iterate_propagates_callback_error); + AKSL_RUN(failures, test_preorder_visits_root_left_right); + AKSL_RUN(failures, test_inorder_visits_left_root_right); + AKSL_RUN(failures, test_postorder_visits_left_right_root); + AKSL_RUN(failures, test_dfs_is_an_alias_for_preorder); + AKSL_RUN(failures, test_bfs_visits_level_by_level); + AKSL_RUN(failures, test_bfs_right_visits_right_child_first); + AKSL_RUN(failures, test_visit_mode_does_not_descend); + + AKSL_RUN(failures, test_custom_allocator_is_used_and_balanced); + AKSL_RUN(failures, test_break_during_bfs_releases_the_queue); + + AKSL_RUN(failures, test_single_node_tree_visits_once_in_every_order); + AKSL_RUN(failures, test_degenerate_chains); + AKSL_RUN(failures, test_tree_deeper_than_the_cap_is_out_of_bounds); + AKSL_RUN(failures, test_cyclic_tree_is_caught); + + AKSL_RUN(failures, test_break_aborts_the_whole_traversal); + AKSL_RUN(failures, test_callback_error_propagates); + + AKSL_RUN(failures, test_null_arguments); + AKSL_RUN(failures, test_unknown_searchmode_is_a_value_error); AKSL_REPORT(failures); } diff --git a/tests/test_tree_iterate_break.c b/tests/test_tree_iterate_break.c deleted file mode 100644 index 6b811cb..0000000 --- a/tests/test_tree_iterate_break.c +++ /dev/null @@ -1,89 +0,0 @@ -/* - * KNOWN FAILING -- TODO.md section 2.1.3 - * - * AKERR_ITERATOR_BREAK does not abort a tree traversal. aksl_tree_iterate - * recurses into itself, and the frame in which the callback raises the break - * handles it in its own PROCESS/HANDLE(e, AKERR_ITERATOR_BREAK) block and - * returns *success*. The parent frame's PASS therefore sees no error at all and - * carries straight on into the sibling subtree, so the traversal runs to - * completion. - * - * The 7-node tree below is walked pre-order (0, 1, 3, 4, 2, 5, 6) with the - * callback breaking on tree[3], the third node visited. A working break stops - * the walk at 3 visits; today the callback is invoked all 7 times. - * - * tests/test_tree.c cannot catch this: it hides its value in tree[6], which is - * the last node visited in all three depth-first orders, so a break that never - * fires is indistinguishable from one that fires on the final node. - * - * Registered in AKSL_KNOWN_FAILING_TESTS (WILL_FAIL). When the break semantics - * are fixed, CTest reports this as unexpectedly passing -- move it into - * AKSL_TESTS then. - */ - -#include "aksl_capture.h" - -#define MAX_LEAVES 7 - -typedef struct BreakParams -{ - aksl_TreeNode *stop_at; - int visits; -} BreakParams; - -static akerr_ErrorContext AKERR_NOIGNORE *break_at_node(aksl_TreeNode *node, void *data) -{ - BreakParams *parms = NULL; - - PREPARE_ERROR(e); - FAIL_ZERO_RETURN(e, node, AKERR_NULLPOINTER, "node"); - FAIL_ZERO_RETURN(e, data, AKERR_NULLPOINTER, "data"); - parms = (BreakParams *)data; - parms->visits += 1; - if ( node == parms->stop_at ) { - FAIL_RETURN(e, AKERR_ITERATOR_BREAK, "stop"); - } - SUCCEED_RETURN(e); -} - -static int test_break_aborts_the_whole_traversal(void) -{ - aksl_TreeNode tree[MAX_LEAVES]; - BreakParams parms; - - memset((void *)tree, 0x00, sizeof(tree)); - memset((void *)&parms, 0x00, sizeof(parms)); - - /* - * TREE[0] - * +--------^^---------+ - * | | - * TREE[1] TREE[2] - * +---^^---+ +---^^---+ - * | | | | - *TREE[3] TREE[4] TREE[5] TREE[6] - */ - tree[0].left = &tree[1]; - tree[0].right = &tree[2]; - tree[1].left = &tree[3]; - tree[1].right = &tree[4]; - tree[2].left = &tree[5]; - tree[2].right = &tree[6]; - - parms.stop_at = &tree[3]; - - AKSL_CHECK_OK(aksl_tree_iterate(&tree[0], &break_at_node, NULL, NULL, - AKSL_TREE_SEARCH_DFS_PREORDER, &parms, NULL)); - /* Pre-order visits 0, 1, 3 -- the break on tree[3] must stop the walk there. */ - AKSL_CHECK(parms.visits == 3); - return 0; -} - -int main(void) -{ - int failures = 0; - - akerr_init(); - AKSL_RUN(failures, test_break_aborts_the_whole_traversal); - AKSL_REPORT(failures); -}