Every libakgl test suite could report success while failing. libakerror's default unhandled-error handler ended in exit(errctx->status), an exit status is one byte wide, and libakgl's band starts at 256 -- so AKGL_ERR_SDL, the most common failure a library built on SDL can have, exited 0 and CTest recorded a pass. tests/character.c aborted at its second of four tests on a bad renderer and was green for months. 0.5.0 worked around that here with TEST_TRAP_UNHANDLED_ERRORS() in tests/testutil.h, and TODO.md ended the entry saying any consumer's suites have the same problem and it was worth raising upstream. It was. 2.0.1 fixes it at the source: akerr_exit() owns the mapping and the default handler calls it, so 0 exits 0, 1 through 255 exit themselves, and anything else exits AKERR_EXIT_STATUS_UNREPRESENTABLE (125). The trap and its 21 call sites are gone. Verified by putting the original failure back rather than by reading the release notes: a FAIL_BREAK(AKGL_ERR_SDL) in tests/character.c's main exits 125 and CTest reports a failure. A standalone consumer raising the same status unhandled exits 125 where it exited 0 before. tests/actor.c installs its own handler and called exit(errctx->status) from it, which is the same defect one layer up. It calls akerr_exit() now. 2.0.0 also makes the error pool and the status registry thread safe, which libakgl needs more than it knew: audio_stream_callback raises error contexts on SDL's audio thread. With an unlocked pool that callback and the main thread could scan AKERR_ARRAY_ERROR at the same time and be handed the same slot. The comment there says so. This is a hard dependency floor, not a preference. 2.0.0 moved __akerr_last_ignored to thread-local storage and made akerr_next_error() return a context that already holds its reference, and both expand at libakgl's call sites -- and at a consumer's, because akerror.h is part of libakgl's public interface. Mixing headers and libraries across that line double-counts every reference and never returns a pool slot. The soname moved to libakerror.so.2; include/akgl/error.h now also feature- tests AKERR_EXIT_STATUS_UNREPRESENTABLE, which is the narrowest probe for 2.0.1 since libakerror publishes no version macro. 0.7.0 for that reason: libakgl's own ABI is unchanged, but the one it re-exports through its headers is not. TODO.md records the pkg-config gap this makes sharper -- akgl.pc names no dependencies at all, so nothing tells a pkg-config consumer which libakerror it needs. Clean build, 26/26 ctest, memcheck clean, warning-clean at -Wall -Werror. libakgl.so.0.7 links libakerror.so.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8T5FAYXE8HEJqFLCYwNNc
301 lines
10 KiB
C
301 lines
10 KiB
C
#include <SDL3/SDL.h>
|
|
#include <stdlib.h>
|
|
#include <string.h>
|
|
#include <akerror.h>
|
|
#include <akgl/error.h>
|
|
#include <akgl/heap.h>
|
|
#include <akgl/registry.h>
|
|
#include <akgl/actor.h>
|
|
#include <akgl/staticstring.h>
|
|
|
|
#include "testutil.h"
|
|
|
|
typedef akerr_ErrorContext *(*RegistryFuncPtr)(void);
|
|
|
|
void *sdl_calloc_always_fails(size_t a, size_t b)
|
|
{
|
|
// This forces SDL to simulate an out of memory condition
|
|
return NULL;
|
|
}
|
|
|
|
akerr_ErrorContext *test_akgl_registry_init(RegistryFuncPtr funcptr)
|
|
{
|
|
SDL_malloc_func malloc_func;
|
|
SDL_calloc_func calloc_func;
|
|
SDL_realloc_func realloc_func;
|
|
SDL_free_func free_func;
|
|
|
|
SDL_GetMemoryFunctions(
|
|
&malloc_func,
|
|
&calloc_func,
|
|
&realloc_func,
|
|
&free_func
|
|
);
|
|
PREPARE_ERROR(errctx);
|
|
ATTEMPT {
|
|
SDL_SetMemoryFunctions(
|
|
malloc_func,
|
|
(SDL_calloc_func)&sdl_calloc_always_fails,
|
|
realloc_func,
|
|
free_func
|
|
);
|
|
CATCH(errctx, funcptr());
|
|
} CLEANUP {
|
|
SDL_SetMemoryFunctions(
|
|
malloc_func,
|
|
calloc_func,
|
|
realloc_func,
|
|
free_func
|
|
);
|
|
} PROCESS(errctx) {
|
|
} FINISH(errctx, true);
|
|
|
|
FAIL_RETURN(errctx, AKGL_ERR_BEHAVIOR, "SDL memory allocator fails but registry reports successful property creation");
|
|
}
|
|
|
|
akerr_ErrorContext *test_akgl_registry_init_creation_failures(void)
|
|
{
|
|
PREPARE_ERROR(errctx);
|
|
ATTEMPT {
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_actor));
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
|
printf("Sucess\n");
|
|
} FINISH(errctx, true);
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_sprite));
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
|
printf("Sucess\n");
|
|
} FINISH(errctx, true);
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_spritesheet));
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
|
printf("Sucess\n");
|
|
} FINISH(errctx, true);
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_character));
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
|
printf("Sucess\n");
|
|
} FINISH(errctx, true);
|
|
SUCCEED_RETURN(errctx);
|
|
}
|
|
|
|
/**
|
|
* @brief akgl_get_property must copy the value, and not a byte more.
|
|
*
|
|
* It used to copy a fixed AKGL_MAX_STRING_LENGTH bytes out of whatever SDL
|
|
* returned, and what SDL returns is its own strdup of the value -- four bytes
|
|
* for "0.0". That read up to 4 KiB past the end of somebody else's allocation on
|
|
* every call, which valgrind reported twelve times over in tests/physics.c
|
|
* alone.
|
|
*
|
|
* The destination is filled with a sentinel first, so the assertion is not
|
|
* "the value arrived" -- that would pass either way -- but "the bytes past the
|
|
* terminator were left alone", which is only true if the copy was bounded by
|
|
* the source.
|
|
*/
|
|
akerr_ErrorContext *test_akgl_get_property_copies_only_the_value(void)
|
|
{
|
|
PREPARE_ERROR(errctx);
|
|
akgl_String *value = NULL;
|
|
char oversized[AKGL_MAX_STRING_LENGTH + 8];
|
|
size_t valuelen = 0;
|
|
size_t i = 0;
|
|
bool untouched = true;
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, akgl_heap_init());
|
|
CATCH(errctx, akgl_registry_init_properties());
|
|
CATCH(errctx, akgl_heap_next_string(&value));
|
|
|
|
memset(&value->data, 0x5a, AKGL_MAX_STRING_LENGTH);
|
|
CATCH(errctx, akgl_set_property("registry.short", "0.0"));
|
|
CATCH(errctx, akgl_get_property("registry.short", &value, "unset"));
|
|
|
|
valuelen = strlen("0.0");
|
|
TEST_ASSERT(errctx, strcmp(value->data, "0.0") == 0,
|
|
"akgl_get_property returned \"%s\", expected \"0.0\"", value->data);
|
|
for ( i = (valuelen + 1); i < AKGL_MAX_STRING_LENGTH; i++ ) {
|
|
TEST_ASSERT_FLAG(untouched, (value->data[i] == 0x5a));
|
|
}
|
|
TEST_ASSERT(errctx, untouched,
|
|
"akgl_get_property wrote past the end of a %zu byte value", valuelen);
|
|
|
|
// The default takes the same path as a stored value.
|
|
CATCH(errctx, akgl_get_property("registry.absent", &value, "fallback"));
|
|
TEST_ASSERT(errctx, strcmp(value->data, "fallback") == 0,
|
|
"akgl_get_property returned \"%s\" for an unset property, expected the default",
|
|
value->data);
|
|
|
|
// A value too long for an akgl_String is refused rather than truncated
|
|
// into an unterminated buffer.
|
|
memset(&oversized, 'x', sizeof(oversized));
|
|
oversized[AKGL_MAX_STRING_LENGTH + 7] = '\0';
|
|
CATCH(errctx, akgl_set_property("registry.oversized", (char *)&oversized));
|
|
TEST_EXPECT_STATUS(errctx, AKERR_OUTOFBOUNDS,
|
|
akgl_get_property("registry.oversized", &value, "unset"),
|
|
"reading a property longer than an akgl_String");
|
|
|
|
// And the documented refusal survives: unset, with no default.
|
|
TEST_EXPECT_STATUS(errctx, AKERR_NULLPOINTER,
|
|
akgl_get_property("registry.absent", &value, NULL),
|
|
"reading an unset property with no default");
|
|
} CLEANUP {
|
|
if ( value != NULL ) {
|
|
IGNORE(akgl_heap_release_string(value));
|
|
}
|
|
} PROCESS(errctx) {
|
|
} FINISH(errctx, true);
|
|
SUCCEED_RETURN(errctx);
|
|
}
|
|
|
|
/**
|
|
* @brief Every actor-state bit must be nameable, and every name must map to its own bit.
|
|
*
|
|
* akgl_registry_init_actor_state_strings walks AKGL_ACTOR_STATE_STRING_NAMES
|
|
* and registers entry `i` under the value `1 << i`. Character JSON resolves
|
|
* state names through that registry, so an entry that disagrees with the
|
|
* `#define` in actor.h is a state no character can ever bind a sprite to. Bits
|
|
* 11 and 12 were `UNDEFINED_11` and `UNDEFINED_12` here while actor.h called
|
|
* them MOVING_IN and MOVING_OUT, so both were unreachable by name.
|
|
*
|
|
* The loop also pins the array's length: it reads exactly
|
|
* AKGL_ACTOR_MAX_STATES entries, which is what the header declares and what
|
|
* the definition is now sized by. It was declared one longer than defined.
|
|
*/
|
|
akerr_ErrorContext *test_actor_state_string_names(void)
|
|
{
|
|
PREPARE_ERROR(errctx);
|
|
int i = 0;
|
|
int j = 0;
|
|
Sint64 registered = 0;
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, akgl_registry_init_actor_state_strings());
|
|
|
|
for ( i = 0; i < AKGL_ACTOR_MAX_STATES; i++ ) {
|
|
TEST_ASSERT(errctx, AKGL_ACTOR_STATE_STRING_NAMES[i] != NULL,
|
|
"actor state bit %d has no name", i);
|
|
|
|
registered = SDL_GetNumberProperty(
|
|
AKGL_REGISTRY_ACTOR_STATE_STRINGS,
|
|
AKGL_ACTOR_STATE_STRING_NAMES[i],
|
|
0);
|
|
TEST_ASSERT(errctx, registered == (Sint64)(1 << i),
|
|
"state name %s resolves to %lld, expected %d",
|
|
AKGL_ACTOR_STATE_STRING_NAMES[i],
|
|
(long long)registered,
|
|
(1 << i));
|
|
|
|
// No two entries may share a name, or the later one silently
|
|
// overwrites the earlier in the registry and one bit becomes
|
|
// unreachable without anything looking wrong.
|
|
for ( j = 0; j < i; j++ ) {
|
|
TEST_ASSERT(errctx,
|
|
strcmp(AKGL_ACTOR_STATE_STRING_NAMES[i],
|
|
AKGL_ACTOR_STATE_STRING_NAMES[j]) != 0,
|
|
"actor state bits %d and %d share the name %s",
|
|
j, i, AKGL_ACTOR_STATE_STRING_NAMES[i]);
|
|
}
|
|
}
|
|
|
|
// The two that were unreachable, named explicitly so a regression reads
|
|
// as what it is rather than as an index.
|
|
TEST_ASSERT(errctx,
|
|
SDL_GetNumberProperty(AKGL_REGISTRY_ACTOR_STATE_STRINGS,
|
|
"AKGL_ACTOR_STATE_MOVING_IN", 0)
|
|
== (Sint64)AKGL_ACTOR_STATE_MOVING_IN,
|
|
"AKGL_ACTOR_STATE_MOVING_IN is not resolvable by name");
|
|
TEST_ASSERT(errctx,
|
|
SDL_GetNumberProperty(AKGL_REGISTRY_ACTOR_STATE_STRINGS,
|
|
"AKGL_ACTOR_STATE_MOVING_OUT", 0)
|
|
== (Sint64)AKGL_ACTOR_STATE_MOVING_OUT,
|
|
"AKGL_ACTOR_STATE_MOVING_OUT is not resolvable by name");
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} FINISH(errctx, true);
|
|
SUCCEED_RETURN(errctx);
|
|
}
|
|
|
|
/**
|
|
* @brief Re-initializing a registry must replace the old set, not abandon it.
|
|
*
|
|
* Only akgl_registry_init_actor destroyed the set it was replacing; the other
|
|
* seven leaked an SDL_PropertiesID on every call after the first. A game that
|
|
* resets between levels calls these repeatedly.
|
|
*
|
|
* Also checks that akgl_registry_init() brings up the properties registry.
|
|
* It did not until 0.5.0, so akgl_set_property was a silent no-op for any
|
|
* caller that had not also gone through akgl_game_init.
|
|
*/
|
|
akerr_ErrorContext *test_akgl_registry_init_is_repeatable(void)
|
|
{
|
|
PREPARE_ERROR(errctx);
|
|
akgl_String *readback = NULL;
|
|
|
|
ATTEMPT {
|
|
CATCH(errctx, akgl_registry_init());
|
|
TEST_ASSERT(errctx, AKGL_REGISTRY_PROPERTIES != 0,
|
|
"akgl_registry_init did not create the properties registry");
|
|
|
|
// Whether the old set was *destroyed* is not observable from here --
|
|
// SDL hands out no liveness query, and it reuses ids -- so that half is
|
|
// the memcheck run's job, and `cmake --build build --target memcheck`
|
|
// gates on it. What is observable is that re-initializing really does
|
|
// replace: a key written before the call must be gone after it.
|
|
SDL_SetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 1);
|
|
TEST_ASSERT(errctx,
|
|
SDL_GetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 0) == 1,
|
|
"could not write a sentinel into the sprite registry");
|
|
CATCH(errctx, akgl_registry_init_sprite());
|
|
TEST_ASSERT(errctx, AKGL_REGISTRY_SPRITE != 0,
|
|
"re-initializing the sprite registry left it unset");
|
|
TEST_ASSERT(errctx,
|
|
SDL_GetNumberProperty(AKGL_REGISTRY_SPRITE, "akgl_test_sentinel", 0) == 0,
|
|
"re-initializing the sprite registry kept the old contents");
|
|
|
|
// Configuration set through the registry has to survive and read back,
|
|
// which is the behaviour the missing initializer was breaking.
|
|
CATCH(errctx, akgl_registry_init());
|
|
TEST_EXPECT_OK(errctx, akgl_set_property("test.property", "1234"),
|
|
"setting a property after akgl_registry_init");
|
|
TEST_EXPECT_OK(errctx, akgl_get_property("test.property", &readback, NULL),
|
|
"reading a property back after akgl_registry_init");
|
|
TEST_ASSERT(errctx, strcmp((char *)&readback->data, "1234") == 0,
|
|
"property read back as '%s', expected '1234'",
|
|
(char *)&readback->data);
|
|
} CLEANUP {
|
|
if ( readback != NULL ) {
|
|
IGNORE(akgl_heap_release_string(readback));
|
|
}
|
|
} PROCESS(errctx) {
|
|
} FINISH(errctx, true);
|
|
SUCCEED_RETURN(errctx);
|
|
}
|
|
|
|
int main(void)
|
|
{
|
|
PREPARE_ERROR(errctx);
|
|
ATTEMPT {
|
|
CATCH(errctx, akgl_error_init());
|
|
CATCH(errctx, test_akgl_registry_init_creation_failures());
|
|
CATCH(errctx, test_akgl_get_property_copies_only_the_value());
|
|
CATCH(errctx, test_actor_state_string_names());
|
|
CATCH(errctx, test_akgl_registry_init_is_repeatable());
|
|
} CLEANUP {
|
|
} PROCESS(errctx) {
|
|
} FINISH_NORETURN(errctx);
|
|
|
|
|
|
}
|