Andrew Kesterson
4d80664cd9
TODO.md item 37 said the cast sweep buys nothing until the build turns on the warnings those casts suppress. This is that precondition, and the argument turned out to be right with evidence: -Wall found three genuine signedness mismatches in sprite.c, where uint32_t width, height and speed were passed straight to akgl_get_json_integer_value(..., int *). A cast would have silenced all three. Fixing them properly also turned up that speed is scaled by a million into a 32-bit field, so anything past 4294 ms overflowed rather than being held. The other real finding was ten -Wstringop-truncation warnings, every one a fixed-width strncpy. Those leave a name field unterminated when the input fills it -- and those fields are registry keys, handed to strcmp and SDL property calls that do not stop at the field. Same overread class fixed twice already this release. All ten now use aksl_strncpy from akstdlib, which always terminates and never NUL-pads, bounded to size - 1 so an over-long name still truncates rather than being refused. staticstring.h documented the old strncpy semantics explicitly and has been rewritten to match. Two sites in game.c are deliberately left on strncpy: both stage into a pre-zeroed buffer for the savegame's fixed-width fields, where the NUL padding is part of the format. Neither is flagged. sprite.c also had a memcpy of a full 128-byte field from a NUL-terminated string, which reads past the end of any shorter name. Not flagged by anything; found while converting its neighbours. -Werror is an option, default OFF, on only in the cmake_build and performance CI jobs. libakgl is consumed with add_subdirectory -- akbasic does exactly that -- so a target-level -Werror turns every diagnostic a newer compiler invents into a broken build for somebody else. The warning set is not even stable across build types here: an -O2 build reports ten warnings that -O0 and -fsyntax-only do not, because they need the middle end. The two jobs that set it are one of each. -Wextra is deliberately not adopted. It adds 22 findings, 17 inherent to the design: 13 -Wunused-parameter from backend vtables and SDL callbacks that must match a signature, and 4 -Wimplicit-fallthrough from libakerror's own PROCESS/HANDLE/HANDLE_GROUP, which fall through by design. Filed upstream as deps/libakerror TODO item 8; the submodule bump carries it. deps/semver/semver.c is listed directly in add_library, so the target's PRIVATE options reach it. Exempted with -w so a future semver update cannot fail this build -- verified by forcing -Wextra -Werror and watching our files fail while semver compiled clean. Verified: RelWithDebInfo, Debug and coverage builds all clean under -Werror; -Werror actually fails on a planted unused variable; an embedded consumer with AKGL_WERROR unset still configures and builds. 25/25 pass, memcheck clean, reindent --check, check_api_surface and check_error_protocol clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
78 lines
3.8 KiB
C
78 lines
3.8 KiB
C
/**
|
|
* @file staticstring.h
|
|
* @brief A fixed-capacity string object handed out by the akgl string heap layer.
|
|
*
|
|
* The library allocates nothing at runtime, so a "string" here is a
|
|
* PATH_MAX-sized buffer claimed from the pool with akgl_heap_next_string and
|
|
* given back with akgl_heap_release_string. Capacity is fixed at compile time:
|
|
* these functions truncate rather than grow, and truncation is silent.
|
|
*/
|
|
|
|
#ifndef _AKGL_STATICSTRING_H_
|
|
#define _AKGL_STATICSTRING_H_
|
|
|
|
#include <string.h>
|
|
#include <akerror.h>
|
|
#include <limits.h>
|
|
|
|
#define AKGL_MAX_STRING_LENGTH PATH_MAX
|
|
|
|
/** @brief Provides a fixed-capacity, heap-managed string buffer. */
|
|
typedef struct
|
|
{
|
|
int refcount; /**< Pool bookkeeping; 0 means the slot is free. Owned by the heap layer. */
|
|
char data[AKGL_MAX_STRING_LENGTH]; /**< The characters. Not guaranteed NUL-terminated when filled to capacity. */
|
|
} akgl_String;
|
|
|
|
/**
|
|
* @brief Set a pooled string's contents and mark the slot in use.
|
|
*
|
|
* Copies @p init into the buffer, or zeroes it when @p init is `NULL`, then
|
|
* sets `refcount` to 1. Callers normally reach this through
|
|
* akgl_heap_next_string rather than calling it directly.
|
|
*
|
|
* @param obj The pooled string to (re)initialize. Required. Its previous
|
|
* contents are discarded without inspection.
|
|
* @param init Initial contents, NUL-terminated. Optional -- `NULL` zero-fills
|
|
* the buffer instead. An @p init that does not fit is truncated,
|
|
* and the result is **always NUL-terminated**. Until 0.5.0 this was
|
|
* `strncpy` at exactly the buffer size, which left an over-long
|
|
* string unterminated -- and a pooled string is handed to `strcmp`,
|
|
* `realpath` and SDL property calls, none of which stop at 4096.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If @p obj is `NULL`.
|
|
*
|
|
* @note Until 0.5.0 the `NULL` @p init path zeroed `sizeof(akgl_String)` bytes
|
|
* starting at `data`, which is four bytes past the end of the buffer --
|
|
* `refcount` sits in front of it, so the overrun landed on the next pool
|
|
* slot's reference count.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_string_initialize(akgl_String *obj, char *init);
|
|
/**
|
|
* @brief Copy the contents of one pooled string into another.
|
|
*
|
|
* A bounded copy between two already-claimed pool slots. It copies bytes only:
|
|
* `refcount` is left alone, so @p dest keeps whatever pool state it had.
|
|
*
|
|
* @param src Source string. Required. Read up to @p count bytes.
|
|
* @param dest Destination string. Required. Overwritten in place; the pool
|
|
* slot must already have been claimed.
|
|
* @param count Maximum bytes to copy. 0 selects #AKGL_MAX_STRING_LENGTH, the
|
|
* whole buffer. A @p count shorter than the source truncates, and
|
|
* the result is **always NUL-terminated**. The remainder is not
|
|
* zero-padded, so this no longer writes the full buffer for a
|
|
* short copy. A negative @p count, or one above
|
|
* #AKGL_MAX_STRING_LENGTH, is refused -- both buffers are exactly
|
|
* that long, so a larger count walked off the end of two pool
|
|
* slots at once.
|
|
* @return `NULL` on success, otherwise an error context owned by the caller.
|
|
* @throws AKERR_NULLPOINTER If @p src or @p dest is `NULL`.
|
|
* @throws AKERR_OUTOFBOUNDS If @p count is negative or above
|
|
* #AKGL_MAX_STRING_LENGTH.
|
|
* @throws AKERR_VALUE, AKERR_OUTOFBOUNDS Whatever `aksl_strncpy` reports. The
|
|
* unreachable `errno` path this used to document went with the
|
|
* `strncpy` call it described.
|
|
*/
|
|
akerr_ErrorContext AKERR_NOIGNORE *akgl_string_copy(akgl_String *src, akgl_String *dest, int count);
|
|
#endif //_AKGL_STATICSTRING_H_
|