Take libakgl 0.7.0, and refuse an over-long type or field name

0.6.0 and 0.7.0 break nothing here: 0.6.0 is three arcade-physics fixes and a
physics.max_timestep property this port does not use, and 0.7.0 reports failures
libakstdlib's wrappers were already catching and takes libakerror 2.0.1 so it
can drop the exit-status trap its own suites needed. That is the same defect
include/akbasic/error.h guards for this band, now fixed at the source rather
than worked around in two places.

The floor moves to 0.7.0 anyway. The soname carries MAJOR.MINOR while the major
is 0, so deciding for ourselves which of libakgl's minor releases were really
compatible is exactly the judgement it exists to take away.

0.7.0 is largely about a defect class worth checking for here rather than
assuming past: ten unterminated strncpy calls into fixed-width name fields. Ours
are clean -- every one is bounded to size - 1 and every buffer is either
explicitly terminated afterwards or memset first, checked site by site. But two
of them, both written last week in src/structtype.c, truncated an over-long name
silently, and truncation is an error everywhere else in this interpreter. It
matters more for a name than for a value: two long type names trimmed to the
same 31 characters collide, and the second is then reported as redeclaring a
type the program never wrote.

Both are refused now, showing the name as written. The check was unreachable as
first drafted, because the prescan read words into a buffer the size of the
limit and so trimmed them before anything could look -- the scratch is twice the
limit now, which is what makes the two cases distinguishable at all.
tests/struct_types.c pins that it stays reachable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

MAINTENANCE.md

@@ -166,7 +166,7 @@ a mixed build leaks pool slots or frees one twice rather than failing to link.
 |---|---|---|---|---|
 | `deps/libakerror` | 2.0.1 | `libakerror.so.2` | major only | **none** — no version macro; `include/akbasic/error.h` feature-tests `AKERR_THREAD_SAFE` and `AKERR_EXIT_STATUS_UNREPRESENTABLE` instead |
 | `deps/libakstdlib` | 0.2.0 | `libakstdlib.so.0.2` | **`MAJOR.MINOR` while major is 0** | `AKSL_VERSION_*`, `aksl_version()`, `AKSL_VERSION_CHECK()` |
-| `deps/libakgl` | 0.5.0 | `libakgl.so.0.5` | **`MAJOR.MINOR` while major is 0** | `AKGL_VERSION*`, `akgl_version()`, `AKGL_VERSION_AT_LEAST()` |
+| `deps/libakgl` | 0.7.0 | `libakgl.so.0.7` | **`MAJOR.MINOR` while major is 0** | `AKGL_VERSION*`, `akgl_version()`, `AKGL_VERSION_AT_LEAST()` |
 
 For both 0.x libraries the soname carries `MAJOR.MINOR` deliberately: 0.1 and 0.2 are
 *different* ABIs, and both become major-only at 1.0. Do not read `0.1 → 0.2` as a compatible

README.md

@@ -144,7 +144,7 @@ nothing to install first.
 * [libakstdlib](https://source.starfort.tech/andrew/libakstdlib) 0.2.0 — libc wrappers that
   report through `libakerror`. String-to-number conversion goes straight to it, which is why
   `VAL("garbage")` is an error rather than a silent `0`.
-* [libakgl](https://source.starfort.tech/andrew/libakgl) 0.5.0 — **optional**, only for
+* [libakgl](https://source.starfort.tech/andrew/libakgl) 0.7.0 — **optional**, only for
   `-DAKBASIC_WITH_AKGL=ON`. Pulls in SDL3. Its soname carries `MAJOR.MINOR` while the major is 0,
   so rebuild rather than relink.
 * [basicinterpret](https://source.starfort.tech/andrew/basicinterpret) — the Go original.

TODO.md

@@ -1911,7 +1911,7 @@ Dependency baseline:
 |---|---|---|
 | `deps/libakerror` | 2.0.1 | Private ownership-enforced status registry. akbasic reserves 512–767 in `akbasic_error_register()`. Does not namespace its `coverage` target when embedded — worked around in our `CMakeLists.txt`. **2.0.0 is thread safe and an ABI break** (`libakerror.so.2`): `__akerr_last_ignored` is thread-local and `akerr_next_error()` returns an owned reference, neither of which fails to link when mismatched. **2.0.1 fixes an exit status that mattered more to this band than to any other** — see below. |
 | `deps/libakstdlib` | 0.2.0 | soname `libakstdlib.so.0.2`. `AKSL_VERSION_CHECK()` asserted in `tests/version_check.c`. This release fixed all six confirmed defects the port was working around — see §1.9, where the bans are now lifted. |
-| `deps/libakgl` | 0.5.0 | soname `libakgl.so.0.5`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. **0.3.0 closed every API gap this port had filed** — see §7 — so the four workarounds §3 used to list are gone. 0.4.0 was a leak-and-overread release that changed no public struct. **0.5.0 is the first that broke our source as well as our ABI**: it namespaced every exported symbol, so `akgl_render_bind2d` is `akgl_render_2d_bind`, `akgl_sprite_sheet_coords_for_frame` is `akgl_spritesheet_coords_for_frame`, the `renderer`/`camera`/`window` globals carry the prefix, and `_akgl_renderer`/`_akgl_camera` are `akgl_default_renderer`/`akgl_default_camera`. `include/akbasic/akgl.h` asserts the 0.5.0 floor. |
+| `deps/libakgl` | 0.7.0 | soname `libakgl.so.0.7`. Owns status codes 256–260. Linked and tested under `-DAKBASIC_WITH_AKGL=ON`, which still defaults OFF so the core library and its whole suite build on a machine with no SDL. **0.3.0 closed every API gap this port had filed** — see §7 — so the four workarounds §3 used to list are gone. 0.4.0 was a leak-and-overread release that changed no public struct. **0.5.0 is the first that broke our source as well as our ABI**: it namespaced every exported symbol, so `akgl_render_bind2d` is `akgl_render_2d_bind`, `akgl_sprite_sheet_coords_for_frame` is `akgl_spritesheet_coords_for_frame`, the `renderer`/`camera`/`window` globals carry the prefix, and `_akgl_renderer`/`_akgl_camera` are `akgl_default_renderer`/`akgl_default_camera`. `include/akbasic/akgl.h` asserts the floor. **0.6.0 and 0.7.0 broke nothing here** — 0.6.0 is three arcade-physics fixes and a `physics.max_timestep` property this port does not use, and 0.7.0 reports failures `libakstdlib`'s wrappers were already catching and takes `libakerror` 2.0.1. The floor moved anyway, because deciding for ourselves which of libakgl's minor releases were really compatible is the judgement the soname exists to take away. |
 
 **An unhandled error in this band used to exit zero, and 512 is the worst possible base for
 that.** `libakerror`'s default unhandled-error handler ended in `exit(errctx->status)`, and a

include/akbasic/akgl.h

@@ -36,10 +36,9 @@
 #include <akgl/version.h>
 
 /*
- * **0.5.0 is a hard floor, API as well as ABI**, which is new: every release
- * before it moved this line for ABI reasons alone. 0.5.0 namespaced every
- * exported symbol, so the spellings this target compiles against do not exist in
- * 0.4.0 and the ones 0.4.0 had do not exist here. `akgl_render_bind2d` is
+ * **0.5.0 was a hard floor, API as well as ABI**, and it is worth keeping the
+ * reason: it namespaced every exported symbol, so the spellings this target
+ * compiles against did not exist before it. `akgl_render_bind2d` is
  * `akgl_render_2d_bind`, `akgl_sprite_sheet_coords_for_frame` is
  * `akgl_spritesheet_coords_for_frame`, and the `renderer`, `camera` and `window`
  * globals carry the prefix -- with `_akgl_renderer` and `_akgl_camera` now
@@ -50,20 +49,27 @@
  * same name, the executable's definition preempted the library's, and every
  * texture load in that suite failed while the suite reported success.
  *
- * The soname carries MAJOR.MINOR while the major is 0, so 0.4 and 0.5 are
+ * **0.6.0 and 0.7.0 broke nothing here**, and the floor moves anyway. 0.6.0 is
+ * three arcade-physics fixes and a `physics.max_timestep` property this target
+ * does not use; 0.7.0 reports failures libakstdlib's wrappers were already
+ * catching, and takes libakerror 2.0.1 so it can drop the exit-status trap its
+ * own suites needed -- the same defect `include/akbasic/error.h` guards for this
+ * band, fixed at the source rather than worked around twice.
+ *
+ * The soname carries MAJOR.MINOR while the major is 0, so 0.5, 0.6 and 0.7 are
  * different ABIs *by declaration* -- libakgl's versioning policy says a 0.x minor
  * bump may break, and the soname is built to match. The floor moves with every
  * one of them, because the alternative is deciding for ourselves which of
  * libakgl's minor releases were really compatible, and that is exactly the
  * judgement the soname exists to take away from us.
  *
- * What it catches is the case the soname cannot: a build against 0.4 headers
- * that happens to find a 0.5 library, or the reverse. Refused here rather than
+ * What it catches is the case the soname cannot: a build against 0.6 headers
+ * that happens to find a 0.7 library, or the reverse. Refused here rather than
  * at link time, because a missing symbol names a function and this names the
  * release.
  */
-#if !AKGL_VERSION_AT_LEAST(0, 5, 0)
-#error "akbasic's libakgl adaptors require libakgl 0.5.0 or later"
+#if !AKGL_VERSION_AT_LEAST(0, 7, 0)
+#error "akbasic's libakgl adaptors require libakgl 0.7.0 or later"
 #endif
 
 #include <akbasic/audio.h>

src/structtype.c

@@ -58,6 +58,18 @@ static const char *skip_lineno(const char *cursor)
     return skip_space(cursor);
 }
 
+/*
+ * Room to read a word that is *too long* and still see that it was.
+ *
+ * Reading into a buffer the size of the limit truncates before anything can
+ * check, so an over-long name would arrive already trimmed to fit and be
+ * accepted -- which is how the length check below was unreachable when it was
+ * first written. Twice the limit is enough to tell the two cases apart, and a
+ * word longer than this is still longer than the limit, so it is refused either
+ * way.
+ */
+#define STRUCT_WORD_SCRATCH	(AKBASIC_MAX_STRUCT_NAME * 2)
+
 /** @brief Copy the next whitespace-delimited word, uppercased. Empty when the line ends. */
 static const char *next_word(const char *cursor, char *dest, size_t len)
 {
@@ -205,8 +217,8 @@ static akerr_ErrorContext *scan_names(akbasic_Runtime *obj)
 {
     PREPARE_ERROR(errctx);
     akbasic_StructTypeTable *table = &obj->structtypes;
-    char word[AKBASIC_MAX_STRUCT_NAME];
-    char name[AKBASIC_MAX_STRUCT_NAME];
+    char word[STRUCT_WORD_SCRATCH];
+    char name[STRUCT_WORD_SCRATCH];
     const char *cursor = NULL;
     int64_t i = 0;
     int open = -1;
@@ -256,8 +268,15 @@ static akerr_ErrorContext *scan_names(akbasic_Runtime *obj)
 	FAIL_ZERO_RETURN(errctx, (table->count < AKBASIC_MAX_STRUCT_TYPES), AKBASIC_ERR_BOUNDS,
 			 "More than %d TYPE declarations", AKBASIC_MAX_STRUCT_TYPES);
 
+	FAIL_ZERO_RETURN(errctx, (strlen(name) < AKBASIC_MAX_STRUCT_NAME), AKBASIC_ERR_BOUNDS,
+			 "TYPE name \"%s\" exceeds the %d character limit",
+			 name, AKBASIC_MAX_STRUCT_NAME - 1);
 	open = table->count;
 	memset(&table->types[open], 0, sizeof(table->types[open]));
+	/* The memset is what terminates this: bounded at size - 1, the last byte
+	   is the zero it already wrote. Refused above rather than truncated,
+	   because two long names truncating to the same prefix would collide
+	   silently -- and truncation is an error everywhere else here. */
 	strncpy(table->types[open].name, name, sizeof(table->types[open].name) - 1);
 	table->types[open].used = true;
 	table->types[open].firstline = i;
@@ -278,8 +297,8 @@ static akerr_ErrorContext *parse_field(akbasic_StructTypeTable *table, akbasic_S
 				       const char *line, int64_t lineno)
 {
     PREPARE_ERROR(errctx);
-    char fieldname[AKBASIC_MAX_STRUCT_NAME];
-    char word[AKBASIC_MAX_STRUCT_NAME];
+    char fieldname[STRUCT_WORD_SCRATCH];
+    char word[STRUCT_WORD_SCRATCH];
     const char *cursor = NULL;
     akbasic_StructField *field = NULL;
     akbasic_Type valuetype = AKBASIC_TYPE_UNDEFINED;
@@ -303,8 +322,13 @@ static akerr_ErrorContext *parse_field(akbasic_StructTypeTable *table, akbasic_S
     }
 
     PASS(errctx, type_from_suffix(fieldname, &valuetype));
+    FAIL_ZERO_RETURN(errctx, (strlen(fieldname) < AKBASIC_MAX_STRUCT_NAME), AKBASIC_ERR_BOUNDS,
+		     "Field name \"%s\" exceeds the %d character limit",
+		     fieldname, AKBASIC_MAX_STRUCT_NAME - 1);
     field = &type->fields[type->fieldcount];
     memset(field, 0, sizeof(*field));
+    /* Terminated by the memset above; refused rather than truncated, for the
+       same reason a type name is. */
     strncpy(field->name, fieldname, sizeof(field->name) - 1);
     field->valuetype = valuetype;
     field->typeindex = -1;

tests/struct_types.c

@@ -218,6 +218,48 @@ static void test_declaration_errors_are_basic_errors(void)
     }
 }
 
+/**
+ * @brief An over-long type or field name is refused, not silently trimmed.
+ *
+ * Truncation is an error everywhere else here, and it matters more for a name
+ * than for a value: two long names trimmed to the same 31 characters would
+ * collide, and the second would be reported as a redeclaration of a type the
+ * program never wrote.
+ *
+ * The check was unreachable when it was first written, because the prescan read
+ * words into a buffer the size of the limit and so trimmed them before anything
+ * could look. Pinned here so it stays reachable.
+ */
+static void test_long_names_are_refused(void)
+{
+    TEST_REQUIRE_OK(harness_start(NULL));
+    TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME,
+					 "10 TYPE AVERYVERYVERYLONGTYPENAMETHATGOESONANDON\n"
+					 "20   W#\n"
+					 "30 END TYPE\n"
+					 "40 PRINT 1\n"));
+    TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
+    TEST_REQUIRE_OK(akbasic_runtime_run(&HARNESS_RUNTIME, 0));
+    TEST_REQUIRE(strstr(HARNESS_OUTPUT, "exceeds the 31 character limit") != NULL,
+		 "an over-long type name should be refused, got \"%s\"", HARNESS_OUTPUT);
+    /* The whole name, not the trimmed one -- that is what makes it actionable. */
+    TEST_REQUIRE(strstr(HARNESS_OUTPUT, "GOESONANDON") != NULL,
+		 "the refusal should show the name as written, got \"%s\"", HARNESS_OUTPUT);
+    harness_stop();
+
+    TEST_REQUIRE_OK(harness_start(NULL));
+    TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME,
+					 "10 TYPE OKNAME\n"
+					 "20   AVERYVERYVERYLONGFIELDNAMETHATGOESON#\n"
+					 "30 END TYPE\n"
+					 "40 PRINT 1\n"));
+    TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
+    TEST_REQUIRE_OK(akbasic_runtime_run(&HARNESS_RUNTIME, 0));
+    TEST_REQUIRE(strstr(HARNESS_OUTPUT, "exceeds the 31 character limit") != NULL,
+		 "an over-long field name should be refused, got \"%s\"", HARNESS_OUTPUT);
+    harness_stop();
+}
+
 int main(void)
 {
     TEST_REQUIRE_OK(akbasic_error_register());
@@ -228,6 +270,7 @@ int main(void)
     test_missing_field_lists_the_others();
     test_self_by_value_refused();
     test_declaration_errors_are_basic_errors();
+    test_long_names_are_refused();
 
     return akbasic_test_failures;
 }