andrew/libakerror
1
0
Fork 0
Code Issues 15 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Use the library's own error idioms inside the library

Browse Source 
Four things in src/error.c did by hand what the macros already do, or
skipped checks the library would have caught for a consumer.

akerr_copy_string() returned void and validated only its capacity, while
writing through a caller-supplied pointer for a caller-supplied length.
It is now __akerr_copy_string() and raises: AKERR_NULLPOINTER for a NULL
destination or source, AKERR_VALUE for a capacity with no room for a
terminator. Both call sites PASS it, and the owner copy in
akerr_reserve_status_range() now gates the commit, so a failed copy
cannot leave a range claimed under an empty owner. It is exported under
the internal prefix rather than static so tests/err_copy_string.c can
drive those guards; nothing else can reach them.

__akerr_name_library_status() and the band reservation in akerr_init()
hand-rolled the log/handler/release sequence. Both now use
ATTEMPT/CATCH/PROCESS/FINISH_NORETURN. PASS does not fit: both sites are
void and have no caller to propagate to, so the terminal form of the same
idiom is the right one -- an unhandled failure prints its stack trace and
goes to akerr_handler_unhandled_error, which terminates, exactly as
before but without the bespoke plumbing. The legacy set path in
akerr_name_for_status() had the same shape and now handles its refusal
with HANDLE_DEFAULT, converting it to the "Unknown Error" sentinel.

Every remaining `if (x) { FAIL_RETURN }` in the registry is now
FAIL_ZERO_RETURN or FAIL_NONZERO_RETURN, and akerr_register_status_name()
checks both owner and name before passing either down --
akerr_store_status_name() reads a NULL owner as "caller did not identify
itself" for the legacy path, so a NULL arriving through the owned entry
point would have skipped the ownership check entirely.

New tests: err_copy_string (the guards above), err_library_status_fatal
(WILL_FAIL -- proves a refused library-status registration terminates).

Tests: ctest 31/31, mutation 80.7% (was 77.5%), line coverage 98.9%.
Branch coverage on src/error.c drops 64.5% -> 50.4%, just over its gate:
each FAIL_* site carries ~6 branch outcomes of error-construction
machinery that only run when that failure fires, and each PASS around a
call that cannot fail carries ~25, so added validation lowers the ratio
by construction. Recorded in TODO.md item 7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Andrew Kesterson <andrew@aklabs.net>
This commit is contained in:
Tachikoma
2026-07-30 21:53:33 -04:00
parent aa953df598
commit 499384ed0d
6 changed files with 255 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

233
src/error.c
View File

@@ -64,13 +64,32 @@ static int akerr_status_range_count;
akerr_ErrorContext AKERR_ARRAY_ERROR[AKERR_MAX_ARRAY_ERROR];
static void akerr_copy_string(char *destination, int capacity, const char *source)
/*
* Bounded copy into a fixed buffer. Every argument is checked: this writes
* through a caller-supplied pointer for a caller-supplied length, so a NULL or
* a non-positive capacity here is a memory error waiting to happen, not
* something to absorb and return from quietly.
*
* Exported under the internal __akerr_ prefix rather than kept static so that
* tests/err_copy_string.c can reach these guards. Both in-library callers
* validate their arguments first, so nothing else can drive them.
*/
akerr_ErrorContext *__akerr_copy_string(char *destination, int capacity,
const char *source)
{
if ( capacity <= 0 ) {
return;
}
PREPARE_ERROR(errctx);
FAIL_NONZERO_RETURN(errctx, (destination == NULL || source == NULL),
AKERR_NULLPOINTER,
"__akerr_copy_string got a NULL %s",
destination == NULL ? "destination buffer" : "source string");
FAIL_NONZERO_RETURN(errctx, (capacity <= 0), AKERR_VALUE,
"__akerr_copy_string got a capacity of %d; a buffer must "
"have room for at least a terminator", capacity);
strncpy(destination, source, (size_t)capacity - 1);
destination[capacity - 1] = '\0';
SUCCEED_RETURN(errctx);
}
/*
@@ -107,27 +126,33 @@ void akerr_default_logger(const char *fmt, ...)
}
/*
* The library naming its own codes. The registry entry points raise errors like
* everything else in the library, but akerr_init() has no caller to raise into,
* so this is where the buck stops: log the failure and take the unhandled-error
* path, which terminates the program.
* The library naming its own codes.
*
* This is fatal on purpose. The library cannot name its own status codes only
* if the build is misconfigured -- a name table too small to hold even the
* library's own entries, or a reservation that did not take -- and the
* akerr_init() returns void and runs before any consumer frame exists, so there
* is nothing to PASS an error to: this *is* the top of the stack. FINISH_NORETURN
* is the library's idiom for that position -- the same one main() uses -- so an
* unhandled failure prints its stack trace and goes to
* akerr_handler_unhandled_error, which terminates.
*
* That is fatal on purpose. The library can only fail to name its own status
* codes if the build is misconfigured -- a name table too small to hold even
* the library's own entries, or a reservation that did not take -- and the
* consequence of continuing is every later stack trace in the process printing
* "Unknown Error" for a code the library defines. That is a startup defect, and
* it is far cheaper to see it at init than to debug it from a degraded trace.
*
* The generated errno table calls this rather than registering names directly,
* so that all of this control flow lives here in one place.
*/
void __akerr_name_library_status(int status, const char *name)
{
akerr_ErrorContext *errctx = akerr_register_status_name(AKERR_LIBRARY_OWNER,
status, name);
if ( errctx != NULL ) {
LOG_ERROR_WITH_MESSAGE(errctx, "** libakerror could not name its own status **");
akerr_handler_unhandled_error(errctx);
RELEASE_ERROR(errctx);
}
PREPARE_ERROR(errctx);
ATTEMPT {
CATCH(errctx, akerr_register_status_name(AKERR_LIBRARY_OWNER, status, name));
} CLEANUP {
} PROCESS(errctx) {
} FINISH_NORETURN(errctx);
}
/*
@@ -161,19 +186,17 @@ void akerr_init()
/* errno and AKERR_* values are the library-owned compatibility band.
* This must precede every registration below: naming a status is only
* permitted inside a reserved range. */
akerr_ErrorContext *reserve_err =
akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT,
AKERR_LIBRARY_OWNER);
if ( reserve_err != NULL ) {
/* Fatal for the same reason as __akerr_name_library_status():
* without this band the library owns nothing, so none of the names
* below can register either. */
LOG_ERROR_WITH_MESSAGE(reserve_err,
"** libakerror could not reserve its own status band **");
akerr_handler_unhandled_error(reserve_err);
RELEASE_ERROR(reserve_err);
}
* permitted inside a reserved range. Terminal for the same reason as
* __akerr_name_library_status(), and handled the same way: without this
* band the library owns nothing, so none of the names below could
* register either. */
PREPARE_ERROR(errctx);
ATTEMPT {
CATCH(errctx, akerr_reserve_status_range(0, AKERR_RESERVED_STATUS_COUNT,
AKERR_LIBRARY_OWNER));
} CLEANUP {
} PROCESS(errctx) {
} FINISH_NORETURN(errctx);
/* Every AKERR_* code gets a name; tests/err_error_names.c asserts the
* list is exhaustive so a new code cannot be added without one. */
@@ -334,55 +357,59 @@ static akerr_ErrorContext AKERR_NOIGNORE *akerr_store_status_name(const char *ow
akerr_StatusName *entry;
PREPARE_ERROR(errctx);
if ( name == NULL ) {
FAIL_RETURN(errctx, AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d for %s: the name is NULL",
status, owner == NULL ? "an unnamed caller" : owner);
}
if ( owner != NULL && ( owner[0] == '\0' ||
strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH ) ) {
FAIL_RETURN(errctx, AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d (\"%s\"): the owner string is "
"empty or longer than %d characters",
status, name, AKERR_MAX_STATUS_RANGE_OWNER_LENGTH - 1);
}
FAIL_NONZERO_RETURN(errctx, (name == NULL), AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d for %s: the name is NULL",
status, owner == NULL ? "an unnamed caller" : owner);
FAIL_NONZERO_RETURN(errctx,
(owner != NULL && ( owner[0] == '\0' ||
strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH )),
AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d (\"%s\"): the owner string "
"is empty or longer than %d characters",
status, name, AKERR_MAX_STATUS_RANGE_OWNER_LENGTH - 1);
range = akerr_range_for_status(status);
if ( range == NULL ) {
FAIL_RETURN(errctx, AKERR_STATUS_NAME_UNRESERVED,
"Refusing to name status %d (\"%s\") for %s: no reserved "
"range contains it. Call akerr_reserve_status_range() first.",
status, name, owner == NULL ? "an unnamed caller" : owner);
}
if ( owner != NULL && strcmp(owner, range->owner) != 0 ) {
FAIL_RETURN(errctx, AKERR_STATUS_NAME_FOREIGN,
"Refusing to name status %d (\"%s\") for %s: that status is "
"in range %d..%d owned by %s.",
status, name, owner,
range->first, range->last, range->owner);
}
FAIL_ZERO_RETURN(errctx, range, AKERR_STATUS_NAME_UNRESERVED,
"Refusing to name status %d (\"%s\") for %s: no reserved "
"range contains it. Call akerr_reserve_status_range() first.",
status, name, owner == NULL ? "an unnamed caller" : owner);
FAIL_NONZERO_RETURN(errctx,
(owner != NULL && strcmp(owner, range->owner) != 0),
AKERR_STATUS_NAME_FOREIGN,
"Refusing to name status %d (\"%s\") for %s: that status "
"is in range %d..%d owned by %s.",
status, name, owner,
range->first, range->last, range->owner);
entry = akerr_status_slot(status, 1);
if ( entry == NULL ) {
FAIL_RETURN(errctx, AKERR_STATUS_NAME_FULL,
"Status name registry is full (%d entries); dropping name "
"\"%s\" for status %d. Rebuild libakerror with a larger "
"AKERR_STATUS_NAME_SLOTS.",
AKERR_MAX_REGISTERED_STATUS_NAMES, name, status);
}
akerr_copy_string(entry->name, AKERR_MAX_ERROR_NAME_LENGTH, name);
FAIL_ZERO_RETURN(errctx, entry, AKERR_STATUS_NAME_FULL,
"Status name registry is full (%d entries); dropping name "
"\"%s\" for status %d. Rebuild libakerror with a larger "
"AKERR_STATUS_NAME_SLOTS.",
AKERR_MAX_REGISTERED_STATUS_NAMES, name, status);
PASS(errctx, __akerr_copy_string(entry->name, AKERR_MAX_ERROR_NAME_LENGTH, name));
SUCCEED_RETURN(errctx);
}
/* Register a name for a status inside a range the caller reserved. */
/*
* Register a name for a status inside a range the caller reserved. Both strings
* are checked here rather than only inside akerr_store_status_name(): the store
* accepts a NULL owner for the legacy akerr_name_for_status() path, so a NULL
* arriving through *this* entry point would be read as "caller did not identify
* itself" and skip the ownership check entirely.
*/
akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, const char *name)
{
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx, (owner != NULL), AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d: the owner string is NULL. "
"Pass the same owner you reserved the range with.",
status);
FAIL_NONZERO_RETURN(errctx, (owner == NULL), AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d: the owner string is NULL. "
"Pass the same owner you reserved the range with.",
status);
FAIL_NONZERO_RETURN(errctx, (name == NULL), AKERR_STATUS_NAME_INVALID,
"Refusing to name status %d for %s: the name is NULL",
status, owner);
PASS(errctx, akerr_store_status_name(owner, status, name));
SUCCEED_RETURN(errctx);
}
@@ -390,9 +417,15 @@ akerr_ErrorContext *akerr_register_status_name(const char *owner, int status, co
/*
* Return or set a name. Status magnitude is unrelated to storage size.
*
* The set path is the legacy two-argument form and cannot hand an error back to
* its caller (it returns a name), so a refusal is logged here and released.
* The set path is the legacy two-argument form. It returns a name, so it cannot
* hand an error back to its caller and cannot raise: it handles the refusal
* here, converting it to the "Unknown Error" sentinel the way any function that
* must return a value converts a caught error into one.
* akerr_register_status_name() is the form that raises.
*
* The lookup path (name == NULL) deliberately stays clear of all of this. FAIL
* calls it to render a status into a stack trace, so it must not itself need an
* error context.
*/
char *akerr_name_for_status(int status, char *name)
{
@@ -400,10 +433,19 @@ char *akerr_name_for_status(int status, char *name)
akerr_init();
if ( name != NULL ) {
akerr_ErrorContext *errctx = akerr_store_status_name(NULL, status, name);
if ( errctx != NULL ) {
PREPARE_ERROR(errctx);
int refused = 0;
ATTEMPT {
CATCH(errctx, akerr_store_status_name(NULL, status, name));
} CLEANUP {
} PROCESS(errctx) {
} HANDLE_DEFAULT(errctx) {
LOG_ERROR_WITH_MESSAGE(errctx, "** REFUSED STATUS NAME **");
RELEASE_ERROR(errctx);
refused = 1;
} FINISH_NORETURN(errctx);
if ( refused != 0 ) {
return "Unknown Error";
}
}
@@ -420,17 +462,17 @@ akerr_ErrorContext *akerr_reserve_status_range(int first_status, int count, cons
int last_status;
PREPARE_ERROR(errctx);
if ( count <= 0 || owner == NULL || owner[0] == '\0' ||
strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH ||
first_status > INT_MAX - (count - 1) ) {
FAIL_RETURN(errctx, AKERR_STATUS_RANGE_INVALID,
"Invalid status range reservation: %d status values from "
"%d for %s (count must be positive, the owner string "
"non-empty and shorter than %d characters, and the range "
"must not overflow int)",
count, first_status, owner == NULL ? "(null)" : owner,
AKERR_MAX_STATUS_RANGE_OWNER_LENGTH);
}
FAIL_NONZERO_RETURN(errctx,
(count <= 0 || owner == NULL || owner[0] == '\0' ||
strlen(owner) >= AKERR_MAX_STATUS_RANGE_OWNER_LENGTH ||
first_status > INT_MAX - (count - 1)),
AKERR_STATUS_RANGE_INVALID,
"Invalid status range reservation: %d status values from "
"%d for %s (count must be positive, the owner string "
"non-empty and shorter than %d characters, and the range "
"must not overflow int)",
count, first_status, owner == NULL ? "(null)" : owner,
AKERR_MAX_STATUS_RANGE_OWNER_LENGTH);
last_status = first_status + count - 1;
for ( int i = 0; i < akerr_status_range_count; i++ ) {
@@ -450,18 +492,21 @@ akerr_ErrorContext *akerr_reserve_status_range(int first_status, int count, cons
akerr_status_ranges[i].owner);
}
}
if ( akerr_status_range_count == AKERR_MAX_RESERVED_STATUS_RANGES ) {
FAIL_RETURN(errctx, AKERR_STATUS_RANGE_FULL,
"Status range table is full (%d ranges); refusing %d..%d "
"for %s.",
AKERR_MAX_RESERVED_STATUS_RANGES,
first_status, last_status, owner);
}
FAIL_NONZERO_RETURN(errctx,
(akerr_status_range_count == AKERR_MAX_RESERVED_STATUS_RANGES),
AKERR_STATUS_RANGE_FULL,
"Status range table is full (%d ranges); refusing %d..%d "
"for %s.",
AKERR_MAX_RESERVED_STATUS_RANGES,
first_status, last_status, owner);
/* The owner copy is what commits the entry, so the count only advances
* once it has succeeded -- a half-written reservation would claim the
* range under an empty owner nobody could ever match. */
akerr_status_ranges[akerr_status_range_count].first = first_status;
akerr_status_ranges[akerr_status_range_count].last = last_status;
akerr_copy_string(akerr_status_ranges[akerr_status_range_count].owner,
AKERR_MAX_STATUS_RANGE_OWNER_LENGTH, owner);
PASS(errctx, __akerr_copy_string(akerr_status_ranges[akerr_status_range_count].owner,
AKERR_MAX_STATUS_RANGE_OWNER_LENGTH, owner));
akerr_status_range_count++;
SUCCEED_RETURN(errctx);
}