2025-08-03 10:07:35 -04:00
|
|
|
#include <SDL3/SDL.h>
|
|
|
|
|
#include <stdlib.h>
|
Fix every memory defect the checker found, and bump to 0.4.0
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's 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 property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:49:11 -04:00
|
|
|
#include <string.h>
|
2026-01-05 08:58:06 -05:00
|
|
|
#include <akerror.h>
|
Fix every memory defect the checker found, and bump to 0.4.0
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's 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 property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:49:11 -04:00
|
|
|
#include <akgl/error.h>
|
|
|
|
|
#include <akgl/heap.h>
|
2026-05-07 22:20:10 -04:00
|
|
|
#include <akgl/registry.h>
|
Fix the type, macro and state-table defects, and the leftover debris
Closes internal-consistency items 19 through 36, 38 and 41. Item 37, the ~180
redundant casts, is deliberately left open with its reasoning in TODO.md: the
benefit only arrives once the build turns on the warnings those casts suppress,
and doing it before that is churn across the two files with the most
outstanding functional defects.
The two that were real bugs are in the actor state table.
AKGL_ACTOR_STATE_STRING_NAMES was declared [AKGL_ACTOR_MAX_STATES+1] and
defined [32], so a consumer trusting the declared bound read past the object;
and indices 11 and 12 were named UNDEFINED_11 and UNDEFINED_12 where actor.h
has MOVING_IN and MOVING_OUT, so no character JSON could bind a sprite to
either state. tests/registry.c now walks the whole table -- every entry
non-NULL, every entry resolving to its own bit, no two entries sharing a name.
The bitmask macros are parenthesized and AKGL_BITMASK_CLEAR has lost the
semicolon inside its body. Writing tests/bitmasks.c for that turned up
something worth knowing: the obvious test does not catch it. For a bit that is
set, the misparse `!(mask & bit) == bit` gives the same answer as the correct
one. It only diverges for an unset bit whose value is not 1, and that is the
shape the suite uses now.
akgl_draw_background was the last public function outside the error protocol.
It takes a backend like everything else in draw.h, restores the draw colour it
found, and is tested -- TODO.md had it filed under "needs the offscreen
renderer harness", which was never true; what it needed was to stop reading the
global.
All eight registry initializers go through one helper, so the seven that leaked
an SDL_PropertiesID on every call after the first no longer do. Fixed in the
same place because it is the same function: akgl_registry_init never called
akgl_registry_init_properties, which made akgl_set_property a silent no-op for
anyone not going through akgl_game_init -- Defects, Known and still open item 3.
Also: AKGL_COLLIDE_RECTANGLES (three open parens, two closes) and akgl_Frame
deleted, float32_t/float64_t used consistently, the developer-specific debug
logging removed from the controller inner loop, the abandoned SDL_GetBasePath
comments removed, nine unused locals removed, and dst renamed to dest.
akgl_game_update's default flags no longer OR the same bit twice. That changes
nothing today, and the reason is Performance item 32: the loop never reads
either bit, which is why every actor is updated sixteen times a frame. Still
open.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 00:15:36 -04:00
|
|
|
#include <akgl/actor.h>
|
Fix every memory defect the checker found, and bump to 0.4.0
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's 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 property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:49:11 -04:00
|
|
|
#include <akgl/staticstring.h>
|
|
|
|
|
|
|
|
|
|
#include "testutil.h"
|
2025-08-03 10:07:35 -04:00
|
|
|
|
2026-05-03 23:57:55 -04:00
|
|
|
typedef akerr_ErrorContext *(*RegistryFuncPtr)(void);
|
2025-08-03 10:07:35 -04:00
|
|
|
|
|
|
|
|
void *sdl_calloc_always_fails(size_t a, size_t b)
|
|
|
|
|
{
|
|
|
|
|
// This forces SDL to simulate an out of memory condition
|
|
|
|
|
return NULL;
|
|
|
|
|
}
|
|
|
|
|
|
2026-05-06 23:18:42 -04:00
|
|
|
akerr_ErrorContext *test_akgl_registry_init(RegistryFuncPtr funcptr)
|
2025-08-03 10:07:35 -04:00
|
|
|
{
|
|
|
|
|
SDL_malloc_func malloc_func;
|
|
|
|
|
SDL_calloc_func calloc_func;
|
|
|
|
|
SDL_realloc_func realloc_func;
|
|
|
|
|
SDL_free_func free_func;
|
Reindent all C sources to the canonical stroustrup style
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:17:37 -04:00
|
|
|
|
2025-08-03 10:07:35 -04:00
|
|
|
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);
|
|
|
|
|
|
2026-07-29 18:01:05 -04:00
|
|
|
FAIL_RETURN(errctx, AKGL_ERR_BEHAVIOR, "SDL memory allocator fails but registry reports successful property creation");
|
2025-08-03 10:07:35 -04:00
|
|
|
}
|
|
|
|
|
|
2026-05-06 23:18:42 -04:00
|
|
|
akerr_ErrorContext *test_akgl_registry_init_creation_failures(void)
|
2025-08-03 10:07:35 -04:00
|
|
|
{
|
|
|
|
|
PREPARE_ERROR(errctx);
|
|
|
|
|
ATTEMPT {
|
2026-05-06 23:18:42 -04:00
|
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_actor));
|
2025-08-03 10:07:35 -04:00
|
|
|
} CLEANUP {
|
|
|
|
|
} PROCESS(errctx) {
|
2026-05-03 23:57:55 -04:00
|
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
Reindent all C sources to the canonical stroustrup style
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:17:37 -04:00
|
|
|
printf("Sucess\n");
|
2025-08-03 10:07:35 -04:00
|
|
|
} FINISH(errctx, true);
|
|
|
|
|
|
|
|
|
|
ATTEMPT {
|
2026-05-06 23:18:42 -04:00
|
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_sprite));
|
2025-08-03 10:07:35 -04:00
|
|
|
} CLEANUP {
|
|
|
|
|
} PROCESS(errctx) {
|
2026-05-03 23:57:55 -04:00
|
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
Reindent all C sources to the canonical stroustrup style
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:17:37 -04:00
|
|
|
printf("Sucess\n");
|
2025-08-03 10:07:35 -04:00
|
|
|
} FINISH(errctx, true);
|
|
|
|
|
|
|
|
|
|
ATTEMPT {
|
2026-05-06 23:18:42 -04:00
|
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_spritesheet));
|
2025-08-03 10:07:35 -04:00
|
|
|
} CLEANUP {
|
|
|
|
|
} PROCESS(errctx) {
|
2026-05-03 23:57:55 -04:00
|
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
Reindent all C sources to the canonical stroustrup style
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:17:37 -04:00
|
|
|
printf("Sucess\n");
|
2025-08-03 10:07:35 -04:00
|
|
|
} FINISH(errctx, true);
|
|
|
|
|
|
|
|
|
|
ATTEMPT {
|
2026-05-06 23:18:42 -04:00
|
|
|
CATCH(errctx, test_akgl_registry_init(&akgl_registry_init_character));
|
2025-08-03 10:07:35 -04:00
|
|
|
} CLEANUP {
|
|
|
|
|
} PROCESS(errctx) {
|
2026-05-03 23:57:55 -04:00
|
|
|
} HANDLE(errctx, AKERR_NULLPOINTER) {
|
Reindent all C sources to the canonical stroustrup style
Mechanical whitespace-only change across src/, include/, tests/, and
util/, applied with scripts/reindent.sh. No behavioral change.
Most files already conformed. The genuine outliers indented at 2 columns
(json_helpers.c, util.c from akgl_rectangle_points onward, assets.c,
staticstring.c, akgl_actor_add_child, util.h, staticstring.h) or used
spaces where the canonical form is a tab (draw.c). cc-mode also re-aligns
line-continuation backslashes in multi-line macros to its default
c-backslash-column, which is required for the tree to be a fixed point of
the indenter.
Verified whitespace-only: `git diff -w` over this change is empty apart
from three trailing blank lines removed at EOF in src/heap.c,
src/registry.c, and tests/tilemap.c. The build produces no new errors and
ctest is unchanged at 13/14, with `character` still the one intentional
failure.
The tree is now a fixed point of `scripts/reindent.sh --check`.
include/akgl/SDL_GameControllerDB.h is generated and excluded.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:17:37 -04:00
|
|
|
printf("Sucess\n");
|
2025-08-03 10:07:35 -04:00
|
|
|
} FINISH(errctx, true);
|
|
|
|
|
SUCCEED_RETURN(errctx);
|
|
|
|
|
}
|
|
|
|
|
|
Fix every memory defect the checker found, and bump to 0.4.0
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's 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 property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:49:11 -04:00
|
|
|
/**
|
|
|
|
|
* @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);
|
|
|
|
|
}
|
|
|
|
|
|
Fix the type, macro and state-table defects, and the leftover debris
Closes internal-consistency items 19 through 36, 38 and 41. Item 37, the ~180
redundant casts, is deliberately left open with its reasoning in TODO.md: the
benefit only arrives once the build turns on the warnings those casts suppress,
and doing it before that is churn across the two files with the most
outstanding functional defects.
The two that were real bugs are in the actor state table.
AKGL_ACTOR_STATE_STRING_NAMES was declared [AKGL_ACTOR_MAX_STATES+1] and
defined [32], so a consumer trusting the declared bound read past the object;
and indices 11 and 12 were named UNDEFINED_11 and UNDEFINED_12 where actor.h
has MOVING_IN and MOVING_OUT, so no character JSON could bind a sprite to
either state. tests/registry.c now walks the whole table -- every entry
non-NULL, every entry resolving to its own bit, no two entries sharing a name.
The bitmask macros are parenthesized and AKGL_BITMASK_CLEAR has lost the
semicolon inside its body. Writing tests/bitmasks.c for that turned up
something worth knowing: the obvious test does not catch it. For a bit that is
set, the misparse `!(mask & bit) == bit` gives the same answer as the correct
one. It only diverges for an unset bit whose value is not 1, and that is the
shape the suite uses now.
akgl_draw_background was the last public function outside the error protocol.
It takes a backend like everything else in draw.h, restores the draw colour it
found, and is tested -- TODO.md had it filed under "needs the offscreen
renderer harness", which was never true; what it needed was to stop reading the
global.
All eight registry initializers go through one helper, so the seven that leaked
an SDL_PropertiesID on every call after the first no longer do. Fixed in the
same place because it is the same function: akgl_registry_init never called
akgl_registry_init_properties, which made akgl_set_property a silent no-op for
anyone not going through akgl_game_init -- Defects, Known and still open item 3.
Also: AKGL_COLLIDE_RECTANGLES (three open parens, two closes) and akgl_Frame
deleted, float32_t/float64_t used consistently, the developer-specific debug
logging removed from the controller inner loop, the abandoned SDL_GetBasePath
comments removed, nine unused locals removed, and dst renamed to dest.
akgl_game_update's default flags no longer OR the same bit twice. That changes
nothing today, and the reason is Performance item 32: the loop never reads
either bit, which is why every actor is updated sixteen times a frame. Still
open.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 00:15:36 -04:00
|
|
|
/**
|
|
|
|
|
* @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);
|
|
|
|
|
}
|
|
|
|
|
|
2025-08-03 10:07:35 -04:00
|
|
|
int main(void)
|
|
|
|
|
{
|
|
|
|
|
PREPARE_ERROR(errctx);
|
|
|
|
|
ATTEMPT {
|
Migrate to the libakerror 1.0.0 status registry
libakerror 1.0.0 replaced the consumer-sized status-name array with a
private registry and made status-code ownership explicit and enforced.
AKERR_MAX_ERR_VALUE and __AKERR_ERROR_NAMES are gone, and the registry
entry points raise akerr_ErrorContext * instead of returning int. See
deps/libakerror/UPGRADING.md.
The break was not only source-level. libakgl's codes sat at
AKERR_LAST_ERRNO_VALUE + 18 through + 22, and 1.0.0 claimed exactly
those five offsets for its own AKERR_STATUS_* registry codes, so every
AKGL_ERR_* was aliasing a libakerror status. HANDLE(e,
AKGL_ERR_LOGICINTERRUPT) at physics.c:222 would have swallowed a
foreign-name refusal.
- Move the band to AKERR_FIRST_CONSUMER_STATUS (256) as fixed offsets,
so a libc that grows an errno cannot move the codes, and add
AKGL_ERR_OWNER, AKGL_ERR_LIMIT and AKGL_ERR_COUNT to describe it.
- Reserve the range and register the names through the owned entry
points, PASS-ing each: these are AKERR_NOIGNORE, and the old
akerr_name_for_status calls discarded failure silently.
- Drop the AKERR_MAX_ERR_VALUE=256 compile definition.
- Guard on AKERR_FIRST_CONSUMER_STATUS in include/akgl/error.h, which
now includes <akerror.h> so the guard is reliable. The embedded build
is fine, but the find_package path can pick up a stale installed
header, and 1.0.0 has an soname, so that pairing is an ABI mismatch
rather than a compile problem. Same guard libakstdlib already carries.
Registration also moves out of akgl_heap_init into a new akgl_error_init
in src/error.c. It was in the heap pool's initializer only because that
was the first thing akgl_game_init called, and the upgrade turned five
fire-and-forget name calls into a library-wide ownership claim that can
fail. That placement was hiding a defect: game.c raises AKGL_ERR_SDL
when SDL_CreateMutex fails, five lines before akgl_heap_init ran, so the
earliest error path in the library was guaranteed to print "Unknown
Error". akgl_error_init is now the first statement in akgl_game_init.
Callers that drive subsystems directly must call akgl_error_init first;
it is idempotent, so ordering it precisely is not required. The eleven
test suites that relied on akgl_heap_init to name their statuses now
call it explicitly, or their failure messages would have degraded to
"Unknown Error".
Add tests/error.c: assert every code reads back its registered name,
that the name table and AKGL_ERR_COUNT agree, that a foreign owner is
refused with AKERR_STATUS_NAME_FOREIGN and AKERR_STATUS_RANGE_OVERLAP,
and that repeating the init is a no-op. That last one is a live
constraint, not a triviality -- libakerror treats only an identical
reservation as a repeat, so a subset or superset raises.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 22:20:28 -04:00
|
|
|
CATCH(errctx, akgl_error_init());
|
2026-05-06 23:18:42 -04:00
|
|
|
CATCH(errctx, test_akgl_registry_init_creation_failures());
|
Fix every memory defect the checker found, and bump to 0.4.0
Six findings, all of them libakgl's, all closed. The memcheck run is clean.
Not one of the four json_load_file calls in src/ was ever matched by a
json_decref, so every asset load abandoned its parsed document: 1.5 KB per
sprite, 2.1 KB per character, 9 KB per load of the 2x2 fixture map, and on
the order of a megabyte for a real one. Each loader now releases it in the
CLEANUP block it already had, on the success path as well as the failure
one. akgl_registry_load_properties needed its loop moved inside the ATTEMPT
block first: props is a borrowed reference into the document and is read
after the block ends, so every exit from that loop leaked the whole tree.
akgl_get_property copied a fixed AKGL_MAX_STRING_LENGTH bytes out of what
SDL handed back, which is SDL's 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 property read, returned whatever was there past the terminator, and
would have faulted on a value that landed at the end of a page. It copies
the value and its terminator now, and refuses a value too long for an
akgl_String rather than truncating it into an unterminated buffer. The
header note that described the overread as a quirk describes correct
behaviour instead.
The four savegame name tables wrote a fixed-width field starting at the
registry key, and SDL sizes that allocation to the name. They read past it
on every entry and put what they found into the save file: up to half a
kilobyte of this process's heap per registered object, in a file a player
might send to somebody. They stage through a zeroed buffer now, and a
negative-array-size typedef fails the build if a table's width ever outgrows
it.
akgl_controller_list_keyboards never freed the array SDL_GetKeyboards
allocated for it.
A font could be opened and published and never handed back -- there was no
way to close one, so a game that changed fonts between scenes leaked ten
kilobytes each time, and loading over a live name leaked the font it
displaced. akgl_text_unloadfont is that missing half, and akgl_text_loadfont
calls it when it replaces a name, after the new font has opened so a failed
reload leaves the caller with the font they had. A new public symbol takes
the version to 0.4.0 and the soname with it: an 0.3 consumer cannot be
handed this library and told it is the same ABI.
tests/registry.c fills a destination with a sentinel and asserts the bytes
past the terminator survive a read, which fails against the old copy.
tests/text.c covers unload, double unload, unloading a name that was never
registered, and replacement closing the displaced font. The JSON releases
have no test of their own and cannot sensibly have one -- nothing in the
public API can observe a jansson refcount -- so the memcheck run is their
test, which is an argument for gating it rather than against.
The two remaining findings are in deps/semver's own unit test, which is
vendored. They are suppressed by function name, so a rewrite of those cases
comes back as a finding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-31 15:49:11 -04:00
|
|
|
CATCH(errctx, test_akgl_get_property_copies_only_the_value());
|
Fix the type, macro and state-table defects, and the leftover debris
Closes internal-consistency items 19 through 36, 38 and 41. Item 37, the ~180
redundant casts, is deliberately left open with its reasoning in TODO.md: the
benefit only arrives once the build turns on the warnings those casts suppress,
and doing it before that is churn across the two files with the most
outstanding functional defects.
The two that were real bugs are in the actor state table.
AKGL_ACTOR_STATE_STRING_NAMES was declared [AKGL_ACTOR_MAX_STATES+1] and
defined [32], so a consumer trusting the declared bound read past the object;
and indices 11 and 12 were named UNDEFINED_11 and UNDEFINED_12 where actor.h
has MOVING_IN and MOVING_OUT, so no character JSON could bind a sprite to
either state. tests/registry.c now walks the whole table -- every entry
non-NULL, every entry resolving to its own bit, no two entries sharing a name.
The bitmask macros are parenthesized and AKGL_BITMASK_CLEAR has lost the
semicolon inside its body. Writing tests/bitmasks.c for that turned up
something worth knowing: the obvious test does not catch it. For a bit that is
set, the misparse `!(mask & bit) == bit` gives the same answer as the correct
one. It only diverges for an unset bit whose value is not 1, and that is the
shape the suite uses now.
akgl_draw_background was the last public function outside the error protocol.
It takes a backend like everything else in draw.h, restores the draw colour it
found, and is tested -- TODO.md had it filed under "needs the offscreen
renderer harness", which was never true; what it needed was to stop reading the
global.
All eight registry initializers go through one helper, so the seven that leaked
an SDL_PropertiesID on every call after the first no longer do. Fixed in the
same place because it is the same function: akgl_registry_init never called
akgl_registry_init_properties, which made akgl_set_property a silent no-op for
anyone not going through akgl_game_init -- Defects, Known and still open item 3.
Also: AKGL_COLLIDE_RECTANGLES (three open parens, two closes) and akgl_Frame
deleted, float32_t/float64_t used consistently, the developer-specific debug
logging removed from the controller inner loop, the abandoned SDL_GetBasePath
comments removed, nine unused locals removed, and dst renamed to dest.
akgl_game_update's default flags no longer OR the same bit twice. That changes
nothing today, and the reason is Performance item 32: the loop never reads
either bit, which is why every actor is updated sixteen times a frame. Still
open.
25/25 pass, memcheck clean, reindent --check clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 00:15:36 -04:00
|
|
|
CATCH(errctx, test_actor_state_string_names());
|
|
|
|
|
CATCH(errctx, test_akgl_registry_init_is_repeatable());
|
2025-08-03 10:07:35 -04:00
|
|
|
} CLEANUP {
|
|
|
|
|
} PROCESS(errctx) {
|
|
|
|
|
} FINISH_NORETURN(errctx);
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
}
|