diff --git a/include/akbasic/runtime.h b/include/akbasic/runtime.h index f828d69..8de41fa 100644 --- a/include/akbasic/runtime.h +++ b/include/akbasic/runtime.h @@ -211,8 +211,12 @@ typedef struct akbasic_Runtime * not exist relative to the working directory, which is what makes a `.bas` * beside its art work from anywhere. Group F's disk verbs will want the * same, which is why it is on the runtime rather than in the sprite state. + * + * Sized to AKBASIC_MAX_SOURCE_PATH_LENGTH, not AKBASIC_MAX_LINE_LENGTH: a + * directory is a filesystem path, not a line of BASIC, and the two do not + * belong to the same budget. */ - char sourcepath[AKBASIC_MAX_LINE_LENGTH]; + char sourcepath[AKBASIC_MAX_SOURCE_PATH_LENGTH]; /* * The armed interrupts, and the environment the one currently running was @@ -365,7 +369,7 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_set_ui(akbasic_Runtime *obj, * @param path Path to the program file, or NULL for none. * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKERR_NULLPOINTER When `obj` is NULL. - * @throws AKBASIC_ERR_BOUNDS When the path is longer than AKBASIC_MAX_LINE_LENGTH. + * @throws AKBASIC_ERR_BOUNDS When the path is longer than AKBASIC_MAX_SOURCE_PATH_LENGTH. */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_set_source_path(akbasic_Runtime *obj, const char *path); diff --git a/include/akbasic/symtab.h b/include/akbasic/symtab.h index c636cf3..63af692 100644 --- a/include/akbasic/symtab.h +++ b/include/akbasic/symtab.h @@ -23,9 +23,17 @@ * serves variables, functions and labels. Capacity is a member; the table is * kept below a 75% load factor by construction because `capacity` counts slots * and the caller's logical maximum is smaller. + * + * 172 rather than the old 256: no caller passes akbasic_symtab_init() anything + * larger than AKBASIC_MAX_VARIABLES (128), and 172 keeps that under the 75% + * load factor the comment above promises (128 / 0.75 = 170.7). + * + * AKBASIC_SYMTAB_MAX_KEY 24 rather than 64: the longest identifier across + * examples/breakout and examples/megademo -- variables, DIMmed arrays, labels + * and DEF FN names alike -- is 11 characters (TITLESCREEN, CLEARPOWERS). */ -#define AKBASIC_SYMTAB_MAX_SLOTS 256 -#define AKBASIC_SYMTAB_MAX_KEY 64 +#define AKBASIC_SYMTAB_MAX_SLOTS 172 +#define AKBASIC_SYMTAB_MAX_KEY 24 typedef struct { diff --git a/include/akbasic/types.h b/include/akbasic/types.h index 97bfad2..49484fb 100644 --- a/include/akbasic/types.h +++ b/include/akbasic/types.h @@ -6,6 +6,7 @@ #ifndef _AKBASIC_TYPES_H_ #define _AKBASIC_TYPES_H_ +#include #include #include @@ -27,15 +28,39 @@ #define AKBASIC_MAX_VALUES 64 #define AKBASIC_MAX_VARIABLES 128 -/* Whole-runtime pools */ -#define AKBASIC_MAX_SOURCE_LINES 9999 -#define AKBASIC_MAX_LINE_LENGTH 256 +/* + * Whole-runtime pools. + * + * AKBASIC_MAX_SOURCE_LINES, AKBASIC_MAX_LINE_LENGTH, AKBASIC_MAX_ARRAY_VALUES, + * AKBASIC_MAX_ENVIRONMENTS and AKBASIC_MAX_FUNCTIONS were cut from their + * original values against measurements taken off examples/breakout and + * examples/megademo, the two most demanding programs this interpreter runs -- + * see the memory-footprint discussion this commit's PR body links. Each is + * sized at roughly 1.5-2x the peak the reference corpus actually reaches, not + * at the peak itself. + * + * AKBASIC_MAX_LINE_LENGTH in particular follows Commodore BASIC's own 80-column + * line limit rather than a measurement, which is why sink_stdio.c now refuses a + * line that fills the buffer with no terminator instead of silently truncating + * it: at 256 bytes that failure mode was theoretical, and at 80 it is not. + * + * AKBASIC_MAX_SOURCE_PATH_LENGTH is its own constant rather than a reuse of + * AKBASIC_MAX_LINE_LENGTH, which is where it used to come from. `sourcepath` + * (runtime.h) holds a directory, not a line of BASIC, and the two ideas do not + * scale together: shrinking the line limit to 80 broke every golden test in + * this checkout, because this repository's own working directory is deeper + * than that. PATH_MAX is the actual bound a filesystem path is subject to, so + * it is the one this borrows. + */ +#define AKBASIC_MAX_SOURCE_LINES 2048 +#define AKBASIC_MAX_LINE_LENGTH 80 /* Commodore BASIC's own line limit */ +#define AKBASIC_MAX_SOURCE_PATH_LENGTH PATH_MAX #define AKBASIC_MAX_ARRAY_DEPTH 64 /* dimensions per array */ #define AKBASIC_MAX_ARRAY_ELEMENTS 1024 /* elements in one array */ -#define AKBASIC_MAX_ARRAY_VALUES 4096 /* array elements across all variables */ +#define AKBASIC_MAX_ARRAY_VALUES 2048 /* array elements across all variables */ #define AKBASIC_MAX_STRING_LENGTH 256 /* see TODO.md 1.2 */ -#define AKBASIC_MAX_ENVIRONMENTS 32 /* new: Go allocated these unbounded */ -#define AKBASIC_MAX_FUNCTIONS 64 /* new: Go used an unbounded map */ +#define AKBASIC_MAX_ENVIRONMENTS 12 /* new: Go allocated these unbounded */ +#define AKBASIC_MAX_FUNCTIONS 8 /* new: Go used an unbounded map */ #define AKBASIC_MAX_LABELS 64 /* new: Go used an unbounded map */ /* * Leaves a DO/LOOP condition may use. Its own small pool rather than a second diff --git a/include/akbasic/variable.h b/include/akbasic/variable.h index 1216585..b4929a8 100644 --- a/include/akbasic/variable.h +++ b/include/akbasic/variable.h @@ -13,12 +13,22 @@ #include +#include #include #include typedef struct { - char name[AKBASIC_MAX_STRING_LENGTH]; + /* + * Sized to AKBASIC_SYMTAB_MAX_KEY, not AKBASIC_MAX_STRING_LENGTH: this name + * only ever gets here by surviving akbasic_symtab_set() first + * (akbasic_environment_create() calls it right after this field is + * populated), and that call refuses anything AKBASIC_SYMTAB_MAX_KEY + * characters or longer with AKBASIC_ERR_BOUNDS. A variable whose name did + * not fit could never exist, so the wider buffer was 232 bytes of headroom + * nothing could ever put a byte into. + */ + char name[AKBASIC_SYMTAB_MAX_KEY]; akbasic_Type valuetype; akbasic_Value *values; /** The pool, or `inlinevalue` for a scalar */ int valuecount; diff --git a/src/runtime.c b/src/runtime.c index 69983ae..0b61e0f 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -301,7 +301,7 @@ akerr_ErrorContext *akbasic_runtime_set_source_path(akbasic_Runtime *obj, const } FAIL_ZERO_RETURN(errctx, (length < sizeof(obj->sourcepath)), AKBASIC_ERR_BOUNDS, "Program path of %zu characters exceeds the %d character limit", - length, AKBASIC_MAX_LINE_LENGTH - 1); + length, AKBASIC_MAX_SOURCE_PATH_LENGTH - 1); PASS(errctx, aksl_memcpy(obj->sourcepath, path, length)); obj->sourcepath[length] = '\0'; SUCCEED_RETURN(errctx); diff --git a/src/sink_stdio.c b/src/sink_stdio.c index 1e50bf3..7c1ab34 100644 --- a/src/sink_stdio.c +++ b/src/sink_stdio.c @@ -74,6 +74,19 @@ static akerr_ErrorContext *stdio_readline(akbasic_TextSink *self, char *dest, si if ( *eof ) { SUCCEED_RETURN(errctx); } + /* + * aksl_fgets(3)'s own contract: a full buffer with no trailing newline is + * how a caller spots a line longer than the buffer, because the rest of it + * is still sitting unread in the stream. Refusing here is what makes that + * true -- without it, the unread remainder is picked up by the *next* + * readline() as if it were its own statement, which does not fail, it just + * runs the wrong program. AKBASIC_MAX_LINE_LENGTH is small enough now that + * this is not a hypothetical: examples/breakout's own longest line used to + * clear the old 256-byte ceiling by more than half. + */ + FAIL_NONZERO_RETURN(errctx, (used == len - 1 && dest[used - 1] != '\n' && dest[used - 1] != '\r'), + AKBASIC_ERR_BOUNDS, + "Source line exceeds the %zu character limit", len - 1); /* * Strip the line terminator. The scanner treats \r and \n as end-of-line * anyway, but leaving them on would make a stored source line differ from diff --git a/tests/value_pool.c b/tests/value_pool.c index 51d7ed7..aec9ff6 100644 --- a/tests/value_pool.c +++ b/tests/value_pool.c @@ -97,9 +97,9 @@ static void test_scoped_loop_counter_costs_nothing(void) /** * @brief The pool is genuinely untouched, not merely large enough. * - * Two hundred scope entries and then the pool's *entire* width -- four arrays, + * Two hundred scope entries and then the pool's *entire* width -- two arrays, * because AKBASIC_MAX_ARRAY_ELEMENTS caps any one of them at 1024 while the pool - * holds 4096. One leaked slot and the fourth `DIM` has nowhere to go, which + * holds 2048. One leaked slot and the second `DIM` has nowhere to go, which * makes this the sharpest of the three and by far the cheapest: the two above * prove a program finishes, this proves nothing at all was spent. The long ones * stay because they are the shape the defect was found in. @@ -111,10 +111,8 @@ static void test_pool_is_untouched_by_scopes(void) "30 NEXT T#\n" "40 DIM A#(1024)\n" "50 DIM B#(1024)\n" - "60 DIM C#(1024)\n" - "70 DIM D#(1024)\n" - "80 D#(1023) = 7\n" - "90 PRINT D#(1023)\n" + "70 B#(1023) = 7\n" + "90 PRINT B#(1023)\n" "100 END\n" "110 LABEL SUBA\n" "120 LOC# = 1\n"