Andrew Kesterson
fb58bb01b0
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>
219 lines
11 KiB
C
219 lines
11 KiB
C
/**
|
|
* @file registry.h
|
|
* @brief The name-to-object lookup tables, and the string configuration store.
|
|
*
|
|
* Nothing in this library is passed around by pointer where a name will do. An
|
|
* actor names the character it instantiates, a character names the sprites it
|
|
* draws, a sprite names the sheet it cuts frames from -- all resolved at load
|
|
* time through these eight registries. That is what lets the whole asset graph
|
|
* be described in JSON files that reference each other by name.
|
|
*
|
|
* They are SDL property sets, not akgl types, so a caller can enumerate one with
|
|
* `SDL_EnumerateProperties` -- which is exactly what akgl_registry_iterate_actor
|
|
* and akgl_character_state_sprites_iterate are for.
|
|
*
|
|
* Every id starts at 0, which SDL treats as "no such property set": reads return
|
|
* the default and writes are silently dropped. So an uninitialized registry does
|
|
* not fail loudly, it fails quietly, and akgl_registry_init has to run first.
|
|
*
|
|
* @warning akgl_registry_init does *not* initialize
|
|
* #AKGL_REGISTRY_PROPERTIES -- akgl_registry_init_properties is a
|
|
* separate call, made by akgl_game_init but by nothing else. A program
|
|
* that builds its own startup path and skips it gets a silently
|
|
* no-op akgl_set_property and an akgl_get_property that always hands
|
|
* back the caller's default, which in turn means akgl_render_init2d
|
|
* and akgl_physics_init_arcade quietly ignore their configuration.
|
|
* TODO.md, "Known and still open" item 3.
|
|
*/
|
|
|
|
#ifndef _REGISTRY_H_
|
|
#define _REGISTRY_H_
|
|
|
|
#include <akgl/error.h>
|
|
#include <akgl/staticstring.h>
|
|
|
|
/** @brief Actor name -> `akgl_Actor *`. Written by akgl_actor_initialize, cleared when an actor's last reference goes. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_ACTOR;
|
|
/** @brief Sprite name -> `akgl_Sprite *`. The `name` field from a sprite JSON. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_SPRITE;
|
|
/** @brief Resolved image path -> `akgl_SpriteSheet *`. Keyed by path so two sprites sharing an image share the texture. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_SPRITESHEET;
|
|
/** @brief Character name -> `akgl_Character *`. What akgl_actor_set_character resolves against. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_CHARACTER;
|
|
/** @brief Actor-state name -> its bit value, as a number. Lets character JSON say "AKGL_ACTOR_STATE_FACE_LEFT" instead of 2. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_ACTOR_STATE_STRINGS;
|
|
/** @brief Font name -> `TTF_Font *`. Names are chosen by the caller of akgl_text_loadfont, not derived from the file. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_FONT;
|
|
/** @brief Music name -> audio handle. Created but not yet populated by anything in the library. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_MUSIC;
|
|
/** @brief Configuration key -> string value. Read through akgl_get_property; everything is a string, including numbers. */
|
|
extern SDL_PropertiesID AKGL_REGISTRY_PROPERTIES;
|
|
|
|
/**
|
|
* @brief Create the seven asset registries, in dependency order.
|
|
*
|
|
* Spritesheet, sprite, character, actor, actor-state-strings, font, music. Call
|
|
* it once at startup, before loading anything.
|
|
*
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate any one of the property sets;
|
|
* the message names which registry failed. The registries created before
|
|
* the failure are left in place rather than torn down.
|
|
*
|
|
* @warning This does not create #AKGL_REGISTRY_PROPERTIES -- see the warning on
|
|
* this file.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init();
|
|
/**
|
|
* @brief Create the music registry.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set, so calling it
|
|
* twice leaks the first one along with everything registered in it.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_music();
|
|
/**
|
|
* @brief Create the font registry.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set. See
|
|
* akgl_registry_init_music.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_font();
|
|
/**
|
|
* @brief Create the actor registry, destroying any previous one first.
|
|
*
|
|
* The one initializer here that cleans up after itself, which is what makes it
|
|
* safe to call between levels: it destroys the old property set before creating
|
|
* the replacement, so the actors from the previous map are unregistered in one
|
|
* step. Note that it destroys the *registry*, not the actors -- releasing those
|
|
* is akgl_heap_release_actor's job.
|
|
*
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set. The old one
|
|
* has already been destroyed at that point, so the registry is left at 0.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_actor();
|
|
/**
|
|
* @brief Create the sprite registry.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set. See
|
|
* akgl_registry_init_music.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_sprite();
|
|
/**
|
|
* @brief Create the spritesheet registry.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set. See
|
|
* akgl_registry_init_music.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_spritesheet();
|
|
/**
|
|
* @brief Create the character registry.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set. See
|
|
* akgl_registry_init_music.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_character();
|
|
/**
|
|
* @brief Create the configuration-property registry.
|
|
*
|
|
* Deliberately separate from akgl_registry_init, because configuration has to be
|
|
* in place before the subsystems that read it start up. akgl_game_init creates
|
|
* it; filling it in -- with akgl_registry_load_properties or akgl_set_property --
|
|
* is the caller's job, and has to happen before akgl_render_init2d or
|
|
* akgl_physics_init_arcade run.
|
|
*
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set.
|
|
* @note Overwrites the existing id without destroying the old set, so every
|
|
* property set before a second call is lost.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_properties();
|
|
/**
|
|
* @brief Load a JSON configuration file into the property registry.
|
|
*
|
|
* Expects a document with a top-level `properties` object whose members are all
|
|
* strings; each becomes one entry. Numbers are configured as strings here and
|
|
* parsed by whoever reads them -- `game.screenwidth` is `"800"`, not `800`.
|
|
*
|
|
* @param fname Path to the JSON document. Required. Used verbatim.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If @p fname is `NULL`, or if the file cannot be
|
|
* opened or does not parse. The message carries jansson's line number
|
|
* and text.
|
|
* @throws AKERR_KEY If the document has no top-level `properties` member.
|
|
* @throws AKERR_TYPE If `properties` is not an object, or if one of its members
|
|
* is not a string.
|
|
* @throws AKGL_ERR_HEAP If the string pool is exhausted.
|
|
*
|
|
* @note Writes into #AKGL_REGISTRY_PROPERTIES, so akgl_registry_init_properties
|
|
* has to have run -- otherwise every entry is silently dropped and this
|
|
* still returns success.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_load_properties(char *fname);
|
|
/**
|
|
* @brief Create the actor-state-name registry and fill it from the name table.
|
|
*
|
|
* Walks `AKGL_ACTOR_STATE_STRING_NAMES` and maps entry `i` to the value `1 << i`,
|
|
* which is what lets a character JSON write `"AKGL_ACTOR_STATE_FACE_LEFT"` and
|
|
* have akgl_character_load_json turn it into a bit.
|
|
*
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If SDL cannot allocate the property set. The
|
|
* individual name registrations are not checked.
|
|
*
|
|
* @note The name table it reads disagrees with actor.h in two places: bits 11
|
|
* and 12 are named `UNDEFINED_11`/`UNDEFINED_12` rather than `MOVING_IN`
|
|
* and `MOVING_OUT`, so those two states cannot be named from JSON at all.
|
|
* TODO.md items 24-26.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_registry_init_actor_state_strings();
|
|
/**
|
|
* @brief Set one configuration property.
|
|
*
|
|
* @param name Property key. Required.
|
|
* @param value Property value. Required. SDL copies it, so the caller's buffer
|
|
* can go away afterwards.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If @p name or @p value is `NULL`.
|
|
*
|
|
* @note The write itself is unchecked. If #AKGL_REGISTRY_PROPERTIES is still 0 --
|
|
* akgl_registry_init_properties never having run -- this discards the
|
|
* value and returns success.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_set_property(char *name, char *value);
|
|
/**
|
|
* @brief Read one configuration property into a pooled string, with a fallback.
|
|
*
|
|
* Absence is not an error: an unset property yields @p def, which is how every
|
|
* caller in the library gets a working default without checking first.
|
|
*
|
|
* @param name Property key. Required.
|
|
* @param dest Receives the value. Required. If `*dest` is `NULL` a string is
|
|
* claimed from the pool for you, so initialize it to `NULL` on the
|
|
* first call; either way the caller releases it with
|
|
* akgl_heap_release_string. If `*dest` is non-`NULL` it is written
|
|
* in place.
|
|
* @param def Value to use when @p name is not set. Effectively required --
|
|
* a `NULL` here on an unset property is an AKERR_NULLPOINTER rather
|
|
* than an empty result.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If @p name or @p dest is `NULL`, or if the property
|
|
* is unset and @p def is `NULL`.
|
|
* @throws AKERR_OUTOFBOUNDS If the stored value is #AKGL_MAX_STRING_LENGTH bytes
|
|
* or longer, which will not fit an akgl_String with its terminator.
|
|
* @throws AKERR_VALUE If the value and the destination overlap in memory.
|
|
* @throws AKGL_ERR_HEAP If `*dest` was `NULL` and the string pool is exhausted.
|
|
*
|
|
* @note Only the value and its terminator are copied. The rest of @p dest keeps
|
|
* whatever the previous holder left there, so read the result as a C
|
|
* string rather than as #AKGL_MAX_STRING_LENGTH bytes.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_get_property(char *name, akgl_String **dest, char *def);
|
|
|
|
#endif //_REGISTRY_H_
|