Reject overlong host type and field names

Co-authored-by: Andrew Kesterson <andrew@starfort.tech>

.githooks/pre-push

@@ -1,99 +0,0 @@
-#!/usr/bin/env bash
-#
-# pre-push: run the cheap local gates before a commit reaches the forge.
-#
-# Install once per clone:
-#
-#     git config core.hooksPath .githooks
-#
-# The AKGL build and mutation harness are opt-in because they are expensive:
-#
-#     AKBASIC_HOOK_AKGL=1 git push
-#     AKBASIC_HOOK_MUTATION=1 git push
-#
-# Bypass everything with Git's escape hatch: git push --no-verify
-
-set -u
-
-ZERO_SHA=0000000000000000000000000000000000000000
-
-root=$(git rev-parse --show-toplevel) || exit 1
-cd "$root" || exit 1
-
-# A push that only deletes refs, or an empty push input, has no new commit to
-# test. Git calls hooks for those pushes too.
-has_updates=0
-while read -r _local_ref local_sha _remote_ref _remote_sha; do
-    if [ "$local_sha" != "$ZERO_SHA" ]; then
-        has_updates=1
-    fi
-done
-if [ "$has_updates" -eq 0 ]; then
-    exit 0
-fi
-
-if [ ! -f deps/libakerror/CMakeLists.txt ]; then
-    echo "pre-push: deps/libakerror is empty. Run:" >&2
-    echo "    git submodule update --init --recursive" >&2
-    exit 1
-fi
-
-builddir="${AKBASIC_HOOK_BUILD_DIR:-$(git rev-parse --git-dir)/akbasic-prepush}"
-mkdir -p "$builddir" || exit 1
-logfile="$builddir/last.log"
-
-# Keep output quiet on success, but retain the complete failing command output
-# so a failed push is actionable without rerunning the gate by hand.
-run() {
-    if ! "$@" >"$logfile" 2>&1; then
-        echo >&2
-        echo "pre-push: FAILED: $*" >&2
-        echo "---------------------------------------------------------------" >&2
-        cat "$logfile" >&2
-        echo "---------------------------------------------------------------" >&2
-        echo "pre-push: push aborted. Use 'git push --no-verify' to override." >&2
-        exit 1
-    fi
-}
-
-if [ -x scripts/cppcheck.sh ]; then
-    if command -v cppcheck > /dev/null 2>&1; then
-        echo "pre-push: cppcheck"
-        run scripts/cppcheck.sh
-    else
-        echo "pre-push: cppcheck not installed, skipping the optional check" >&2
-    fi
-else
-    echo "pre-push: scripts/cppcheck.sh not present, skipping the optional check" >&2
-fi
-
-echo "pre-push: default build + ctest"
-run cmake -S . -B "$builddir/default"
-run cmake --build "$builddir/default" --parallel 2
-run ctest --test-dir "$builddir/default" --output-on-failure
-
-echo "pre-push: sanitizer build + ctest"
-run cmake -S . -B "$builddir/sanitize" -DAKBASIC_SANITIZE=ON
-run cmake --build "$builddir/sanitize" --parallel 2
-run ctest --test-dir "$builddir/sanitize" --output-on-failure
-
-if [ "${AKBASIC_HOOK_AKGL:-0}" = "1" ]; then
-    echo "pre-push: AKGL build + ctest"
-    run cmake -S . -B "$builddir/akgl" -DAKBASIC_WITH_AKGL=ON
-    run cmake --build "$builddir/akgl" --parallel 2
-    run env SDL_VIDEODRIVER=dummy SDL_AUDIODRIVER=dummy \
-        SDL_RENDER_DRIVER=software ctest --test-dir "$builddir/akgl" --output-on-failure
-fi
-
-if [ "${AKBASIC_HOOK_MUTATION:-0}" = "1" ]; then
-    threshold="${AKBASIC_MUTATION_THRESHOLD:-65}"
-    mutants="${AKBASIC_HOOK_MUTANTS:-260}"
-    echo "pre-push: mutation testing, threshold ${threshold}%"
-    run python3 scripts/mutation_test.py \
-        --target src/symtab.c \
-        --max-mutants "$mutants" \
-        --threshold "$threshold"
-fi
-
-echo "pre-push: OK"
-exit 0

README.md

@@ -22,18 +22,6 @@ cmake --build build --parallel
 ctest --test-dir build --output-on-failure
 ```
 
-To run the same fast build, sanitizer and test gates before every push, install the advisory
-pre-push hook once in this clone:
-
-```sh norun
-git config core.hooksPath .githooks
-```
-
-The hook keeps its build trees under `.git/akbasic-prepush`, skips the optional cppcheck gate
-when its script or tool is unavailable, and leaves the AKGL build and mutation harness behind
-`AKBASIC_HOOK_AKGL=1` and `AKBASIC_HOOK_MUTATION=1`. `git push --no-verify` remains the escape
-hatch. `core.hooksPath` is local clone configuration, so CI remains the hard gate.
-
 ```sh norun
 ./build/basic                                          # the REPL
 ./build/basic tests/language/functions.bas             # run a program

docs/16-structures.md

@@ -50,7 +50,7 @@ nothing would say which type it is.
 ```
 
 ```output
-? 40 : PARSE ERROR TYPE POINT: POINT is a reserved word and cannot name a type
+? 10 : PARSE ERROR TYPE POINT: POINT is a reserved word and cannot name a type
 
 ```
 

include/akbasic/host.h

@@ -105,6 +105,8 @@ typedef struct
  * Registering before the script is loaded is the normal case. A host type and a
  * `TYPE` the script declares share one namespace, so a script cannot declare a
  * type the host already registered -- and would be refused if it tried.
+ * Host type and field names are limited to 31 characters, matching
+ * script-declared types and fields.
  *
  * @param obj Object to initialize, inspect, or modify.
  * @param type The host's description of its own struct.
@@ -112,6 +114,7 @@ typedef struct
  * @throws AKERR_NULLPOINTER When either argument is NULL.
  * @throws AKBASIC_ERR_VALUE When a field name carries no type suffix, a nested
  *         type is not registered, or the name is already taken.
+ * @throws AKERR_OUTOFBOUNDS When a type or field name exceeds 31 characters.
  * @throws AKBASIC_ERR_BOUNDS When the type table or a field list is full.
  */
 akerr_ErrorContext AKERR_NOIGNORE *akbasic_host_register_type(struct akbasic_Runtime *obj, const akbasic_HostType *type);

src/data.c

@@ -160,14 +160,21 @@ akerr_ErrorContext *akbasic_data_scan(akbasic_Runtime *obj)
 {
     PREPARE_ERROR(errctx);
     int64_t i = 0;
+    int64_t entry = 0;
 
     FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL runtime in data_scan");
+    FAIL_ZERO_RETURN(errctx, (obj->environment != NULL), AKERR_NULLPOINTER,
+		     "Runtime has no environment; call akbasic_runtime_init() first");
+    entry = obj->environment->lineno;
     PASS(errctx, akbasic_data_state_init(&obj->data_state));
     for ( i = 0; i < AKBASIC_MAX_SOURCE_LINES; i++ ) {
 	if ( obj->source[i].code[0] != '\0' ) {
+	    /* Keep BASIC's error prefix on the source line being prescanned. */
+	    obj->environment->lineno = i;
 	    PASS(errctx, scan_line(&obj->data_state, obj->source[i].code, i));
 	}
     }
+    obj->environment->lineno = entry;
     SUCCEED_RETURN(errctx);
 }
 

src/host.c

@@ -235,16 +235,9 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
 
     dest = &obj->structtypes.types[obj->structtypes.count];
     PASS(errctx, aksl_memset(dest, 0, sizeof(*dest)));
-    /*
-     * Raw snprintf, and a latent defect rather than a settled decision: a host
-     * type name over 31 characters truncates silently here, and two that share a
-     * 31-character prefix then collide in akbasic_structtype_find. scan_names()
-     * in structtype.c already refuses the same case for a script-declared type
-     * with an explicit limit message, so the two paths disagree. Converting this
-     * to aksl_strcpy is the fix and it is a behaviour change on a public
-     * registration call, so it wants its own issue rather than this port.
-     */
-    snprintf(dest->name, sizeof(dest->name), "%s", type->name);
+    /* Host and script registration share the 32-byte-including-NUL limit for
+       both type names and field names. */
+    PASS(errctx, aksl_strcpy(dest->name, sizeof(dest->name), type->name));
     dest->used = true;
     dest->ishost = true;
     dest->hostsize = type->size;
@@ -270,8 +263,7 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
 			 "%s.%s must end in '%c' for the C type it describes",
 			 type->name, src->name, suffix_for(src->kind));
 
-	/* Same silent truncation as the type name above, and the same fix. */
-	snprintf(field->name, sizeof(field->name), "%s", src->name);
+	PASS(errctx, aksl_strcpy(field->name, sizeof(field->name), src->name));
 	field->hostkind = src->kind;
 	field->hostoffset = src->offset;
 	field->hostwidth = src->width;

src/runtime.c

@@ -1576,17 +1576,22 @@ akerr_ErrorContext *akbasic_runtime_scan_labels(akbasic_Runtime *obj)
     PREPARE_ERROR(errctx);
     akbasic_Environment *root = NULL;
     int64_t i = 0;
+    int64_t entry = 0;
 
     FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL runtime in scan_labels");
     FAIL_ZERO_RETURN(errctx, (obj->environment != NULL), AKERR_NULLPOINTER,
 		     "Runtime has no environment; call akbasic_runtime_init() first");
     for ( root = obj->environment; root->parent != NULL; root = root->parent ) {
     }
+    entry = obj->environment->lineno;
     for ( i = 0; i < AKBASIC_MAX_SOURCE_LINES; i++ ) {
 	if ( obj->source[i].code[0] != '\0' ) {
+	    /* Keep BASIC's error prefix on the source line being prescanned. */
+	    obj->environment->lineno = i;
 	    PASS(errctx, scan_line_labels(root, obj->source[i].code, i));
 	}
     }
+    obj->environment->lineno = entry;
     SUCCEED_RETURN(errctx);
 }
 

src/structtype.c

@@ -260,11 +260,13 @@ static akerr_ErrorContext *scan_names(akbasic_Runtime *obj)
     size_t namelen = 0;
     bool matched = false;
     const akbasic_Verb *verb = NULL;
+    int64_t entry = obj->environment->lineno;
 
     for ( i = 0; i < AKBASIC_MAX_SOURCE_LINES; i++ ) {
 	if ( obj->source[i].code[0] == '\0' ) {
 	    continue;
 	}
+	obj->environment->lineno = i;
 	cursor = next_word(skip_lineno(obj->source[i].code), word, sizeof(word));
 
 	PASS(errctx, word_is(word, "END", &matched));
@@ -327,6 +329,7 @@ static akerr_ErrorContext *scan_names(akbasic_Runtime *obj)
 
     FAIL_NONZERO_RETURN(errctx, (open >= 0), AKBASIC_ERR_SYNTAX,
 			"TYPE %s is never closed with END TYPE", table->types[open >= 0 ? open : 0].name);
+    obj->environment->lineno = entry;
     SUCCEED_RETURN(errctx);
 }
 
@@ -440,7 +443,7 @@ static akerr_ErrorContext *parse_field(akbasic_StructTypeTable *table, akbasic_S
  * each other by value, which has no finite size. That is the diagnosis rather
  * than a stack overflow later.
  */
-static akerr_ErrorContext *resolve_sizes(akbasic_StructTypeTable *table)
+static akerr_ErrorContext *resolve_sizes(akbasic_Runtime *runtime, akbasic_StructTypeTable *table)
 {
     PREPARE_ERROR(errctx);
     bool progress = true;
@@ -477,6 +480,7 @@ static akerr_ErrorContext *resolve_sizes(akbasic_StructTypeTable *table)
     }
 
     for ( i = 0; i < table->count; i++ ) {
+	runtime->environment->lineno = table->types[i].firstline;
 	FAIL_NONZERO_RETURN(errctx, (table->types[i].slotcount < 0), AKBASIC_ERR_VALUE,
 			    "TYPE %s contains itself by value, so it has no size. "
 			    "A type may only refer to itself through PTR TO",
@@ -493,9 +497,13 @@ akerr_ErrorContext *akbasic_structtype_scan(akbasic_Runtime *obj)
     akbasic_StructTypeTable *table = NULL;
     int64_t i = 0;
     int t = 0;
+    int64_t entry = 0;
 
     FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL runtime in structtype scan");
+    FAIL_ZERO_RETURN(errctx, (obj->environment != NULL), AKERR_NULLPOINTER,
+		     "Runtime has no environment; call akbasic_runtime_init() first");
     table = &obj->structtypes;
+    entry = obj->environment->lineno;
 
     /*
      * Drop what the *script* declared and keep what the *host* registered.
@@ -525,10 +533,12 @@ akerr_ErrorContext *akbasic_structtype_scan(akbasic_Runtime *obj)
 	    if ( obj->source[i].code[0] == '\0' ) {
 		continue;
 	    }
+	    obj->environment->lineno = i;
 	    PASS(errctx, parse_field(table, type, obj->source[i].code, i));
 	}
     }
 
-    PASS(errctx, resolve_sizes(table));
+    PASS(errctx, resolve_sizes(obj, table));
+    obj->environment->lineno = entry;
     SUCCEED_RETURN(errctx);
 }

tests/hoststruct.c

@@ -208,6 +208,72 @@ static void test_suffix_must_match_the_c_type(void)
     harness_stop();
 }
 
+/** @brief Host names use the same bounded storage as script-declared names. */
+static void test_registration_name_limits(void)
+{
+    static const akbasic_HostField SHORT_FIELD[] = {
+	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234#",
+			   AKBASIC_HOSTFIELD_INT32 )
+    };
+    static const akbasic_HostField LONG_FIELD_A[] = {
+	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A#",
+			   AKBASIC_HOSTFIELD_INT32 )
+    };
+    static const akbasic_HostField LONG_FIELD_B[] = {
+	AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B#",
+			   AKBASIC_HOSTFIELD_INT32 )
+    };
+    static const akbasic_HostType SHORT_TYPE = {
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZ12345", sizeof(test_Enemy), SHORT_FIELD, 1
+    };
+    static const akbasic_HostType LONG_TYPE_A = {
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A", sizeof(test_Enemy), SHORT_FIELD, 1
+    };
+    static const akbasic_HostType LONG_TYPE_B = {
+	"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B", sizeof(test_Enemy), SHORT_FIELD, 1
+    };
+    static const akbasic_HostType LONG_FIELD_TYPE_A = {
+	"LONGFIELDA", sizeof(test_Enemy), LONG_FIELD_A, 1
+    };
+    static const akbasic_HostType LONG_FIELD_TYPE_B = {
+	"LONGFIELDB", sizeof(test_Enemy), LONG_FIELD_B, 1
+    };
+    akerr_ErrorContext *raised = NULL;
+
+	TEST_REQUIRE_OK(harness_start(NULL));
+	TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME, &SHORT_TYPE));
+	harness_stop();
+
+	TEST_REQUIRE_OK(harness_start(NULL));
+	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_A);
+	TEST_REQUIRE(raised != NULL, "an overlong type name must be refused");
+	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
+	test_discard_error(raised);
+	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_B);
+	TEST_REQUIRE(raised != NULL, "a second long type name must fail cleanly");
+	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
+	test_discard_error(raised);
+	harness_stop();
+
+	TEST_REQUIRE_OK(harness_start(NULL));
+	TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME,
+						  &(akbasic_HostType){
+						      "SHORTFIELDS", sizeof(test_Enemy), SHORT_FIELD, 1
+						  }));
+	harness_stop();
+
+	TEST_REQUIRE_OK(harness_start(NULL));
+	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_A);
+	TEST_REQUIRE(raised != NULL, "an overlong field name must be refused");
+	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
+	test_discard_error(raised);
+	raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_B);
+	TEST_REQUIRE(raised != NULL, "a second long field name must fail cleanly");
+	TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
+	test_discard_error(raised);
+	harness_stop();
+}
+
 /**
  * @brief After unbinding, the name is refused rather than read.
  *
@@ -258,6 +324,7 @@ int main(void)
     test_conversion_refuses_rather_than_truncates();
     test_copy_versus_point();
     test_suffix_must_match_the_c_type();
+    test_registration_name_limits();
     test_unbind_refuses_later_reads();
     test_registration_survives_a_rerun();
 

tests/interrupts.c

@@ -16,6 +16,8 @@
  * below.
  */
 
+#include <stdio.h>
+
 #include "harness.h"
 
 /**
@@ -341,6 +343,28 @@ static akerr_ErrorContext AKERR_NOIGNORE *test_prescan_boundaries(void)
     SUCCEED_RETURN(errctx);
 }
 
+/** @brief A full label table reports the line whose label could not be filed. */
+static void test_label_prescan_error_line(void)
+{
+    char source[4096] = "";
+    size_t used = 0;
+    int i = 0;
+
+    for ( i = 1; i <= AKBASIC_MAX_LABELS + 1; i++ ) {
+	used += (size_t)snprintf(source + used, sizeof(source) - used,
+				"%d LABEL L%d\n", i, i);
+    }
+    (void)snprintf(source + used, sizeof(source) - used, "100 PRINT 1\n");
+
+    TEST_REQUIRE_OK(harness_start(NULL));
+    TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME, source));
+    TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
+    TEST_REQUIRE(strstr(HARNESS_OUTPUT, "? 65 : PARSE ERROR") != NULL,
+		 "a full label table should report its source line, got \"%s\"",
+		 HARNESS_OUTPUT);
+    harness_stop();
+}
+
 int main(void)
 {
     PREPARE_ERROR(errctx);
@@ -355,6 +379,7 @@ int main(void)
 	CATCH(errctx, test_undefined_label_is_reported());
 	CATCH(errctx, test_arm_refusals());
 	CATCH(errctx, test_prescan_boundaries());
+	test_label_prescan_error_line();
     } CLEANUP {
     } PROCESS(errctx) {
     } HANDLE_DEFAULT(errctx) {

tests/language/structures/reserved_names.txt

@@ -1,2 +1,2 @@
-? 90 : PARSE ERROR TYPE POINT: POINT is a reserved word and cannot name a type
+? 60 : PARSE ERROR TYPE POINT: POINT is a reserved word and cannot name a type
 

tests/read_data.c

@@ -8,6 +8,7 @@
  * TODO.md section 6.
  */
 
+#include <stdio.h>
 #include <string.h>
 
 #include <akbasic/error.h>
@@ -174,6 +175,34 @@ static void test_colon_ends_data(void)
     harness_stop();
 }
 
+/** @brief DATA overflow reports the line where the item limit was crossed. */
+static void test_data_prescan_error_line(void)
+{
+    char source[4096] = "";
+    size_t used = 0;
+    int line = 0;
+    int item = 0;
+
+    for ( line = 1; line <= 34; line++ ) {
+	used += (size_t)snprintf(source + used, sizeof(source) - used, "%d DATA ", line);
+	for ( item = 0; item < 15; item++ ) {
+	    used += (size_t)snprintf(source + used, sizeof(source) - used,
+				      "%s1", (item == 0 ? "" : ","));
+	}
+	used += (size_t)snprintf(source + used, sizeof(source) - used, "\n");
+    }
+	used += (size_t)snprintf(source + used, sizeof(source) - used, "100 DATA 1,1\n");
+	(void)snprintf(source + used, sizeof(source) - used, "101 DATA 1\n200 PRINT 1\n");
+
+    TEST_REQUIRE_OK(harness_start(NULL));
+    TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME, source));
+    TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
+	TEST_REQUIRE(strstr(HARNESS_OUTPUT, "? 101 : PARSE ERROR") != NULL,
+		 "DATA overflow should report its source line, got \"%s\"",
+		 HARNESS_OUTPUT);
+    harness_stop();
+}
+
 /** @brief A float item fills a float variable with its fractional part intact. */
 static void test_float_items(void)
 {
@@ -227,6 +256,7 @@ int main(void)
     test_type_mismatch();
     test_quoted_items();
     test_colon_ends_data();
+	test_data_prescan_error_line();
     test_float_items();
     test_negative_items();
     return akbasic_test_failures;

tests/struct_types.c

@@ -218,6 +218,22 @@ static void test_declaration_errors_are_basic_errors(void)
     }
 }
 
+/** @brief A field declaration error reports the field's source line. */
+static void test_type_prescan_error_line(void)
+{
+    TEST_REQUIRE_OK(harness_start(NULL));
+    TEST_REQUIRE_OK(akbasic_runtime_load(&HARNESS_RUNTIME,
+					 "10 TYPE RECT\n"
+					 "20 W# EXTRA\n"
+					 "30 END TYPE\n"
+					 "40 PRINT 1\n"));
+    TEST_REQUIRE_OK(akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
+    TEST_REQUIRE(strstr(HARNESS_OUTPUT, "? 20 : PARSE ERROR") != NULL,
+		 "TYPE prescan should report its source line, got \"%s\"",
+		 HARNESS_OUTPUT);
+    harness_stop();
+}
+
 /**
  * @brief An over-long type or field name is refused, not silently trimmed.
  *
@@ -270,6 +286,7 @@ int main(void)
     test_missing_field_lists_the_others();
     test_self_by_value_refused();
     test_declaration_errors_are_basic_errors();
+    test_type_prescan_error_line();
     test_long_names_are_refused();
 
     return akbasic_test_failures;