Naming collision #46

Open
opened 2026-08-05 09:18:47 -04:00 by andrew · 4 comments
Owner

akbasic/src/host.c has a latent collision. A host-registered type name over
31 characters truncates silently, and two sharing a 31-character prefix then
collide in akbasic_structtype_find — where structtype.c refuses the
identical case outright. The two registration paths disagree. Flagged
inline, not fixed: it is a behaviour change on a public call.

Plan

  1. In akbasic_host_register_type (src/host.c:247), replace the raw
    snprintf(dest->name, sizeof(dest->name), "%s", type->name); with
    aksl_strcpy(dest->name, sizeof(dest->name), type->name), following the
    PASS(errctx, ...) convention already used elsewhere in this function.
    This mirrors the script-declared path in scan_names()
    (src/structtype.c:320), which already uses aksl_strcpy and refuses
    rather than truncates.
  2. The same bug, and the same inline "same fix" note, exists a few lines
    further down for field names: src/host.c:274 does
    snprintf(field->name, sizeof(field->name), "%s", src->name); inside the
    field-registration loop, with a comment literally reading "Same silent
    truncation as the type name above, and the same fix." Replace that
    snprintf with aksl_strcpy(field->name, sizeof(field->name), src->name)
    too, following the same PASS(errctx, ...) convention. This mirrors the
    script-declared field path in structtype.c:376 (also aksl_strcpy,
    refuses rather than truncates, with limit AKBASIC_MAX_STRUCT_NAME - 1
    from structtype.c:370-372). Both host.c sites should land together —
    fixing only the type name and leaving the field name truncating would
    just relocate the same collision into field lookups.
  3. This changes public behaviour in two ways: a host embedder that
    registers a type name, or a field name, over 31 characters — which
    previously succeeded silently (with truncation) — will now get an error
    from akbasic_host_register_type. Add a short note to the function's doc
    comment (and CHANGELOG if one exists) calling out that host type names
    and field names are now bounded to 31 characters, matching
    script-declared types and fields.
  4. Add a regression test (wherever host registration is already exercised,
    e.g. a test double registering a akbasic_HostType) covering:
    • a type name of exactly 31 characters (succeeds),
    • a type name of 32+ characters (fails with a clear error, not a
    truncate-and-collide),
    • two type names sharing a 32+ character common prefix that previously
    collided after truncation — confirm they now either both register
    successfully (if under the limit) or both fail cleanly,
    • the same three cases repeated for a field name instead of a type
    name (exactly 31 chars succeeds; 32+ chars fails cleanly; two field
    names on the same type sharing a 32+ character common prefix no
    longer collide).
  5. Verify akbasic_structtype_find still behaves correctly once both type
    registration paths use aksl_strcpy — no change expected there, but the
    collision scenario described in the original comment should no longer be
    reachable. Likewise confirm nothing does prefix/length-sensitive lookup
    on field names post-fix.
  6. Remove both inline comments in host.c describing this as a "latent
    defect rather than a settled decision" (the one at the type-name
    snprintf and the "same fix" note at the field-name snprintf) once the
    fix lands, and replace them with a short note that host and script
    registration now share the same 32-byte-including-NUL limit for both
    type names and field names.

Scope: this issue is the host.c side only (both the type-name and
field-name snprintf sites); no change to structtype.c's scan_names() is
required, since it already does the right thing for both.

akbasic/src/host.c has a latent collision. A host-registered type name over 31 characters truncates silently, and two sharing a 31-character prefix then collide in akbasic_structtype_find — where structtype.c refuses the identical case outright. The two registration paths disagree. Flagged inline, not fixed: it is a behaviour change on a public call. ## Plan 1. In akbasic_host_register_type (src/host.c:247), replace the raw snprintf(dest->name, sizeof(dest->name), "%s", type->name); with aksl_strcpy(dest->name, sizeof(dest->name), type->name), following the PASS(errctx, ...) convention already used elsewhere in this function. This mirrors the script-declared path in scan_names() (src/structtype.c:320), which already uses aksl_strcpy and refuses rather than truncates. 2. The same bug, and the same inline "same fix" note, exists a few lines further down for field names: src/host.c:274 does snprintf(field->name, sizeof(field->name), "%s", src->name); inside the field-registration loop, with a comment literally reading "Same silent truncation as the type name above, and the same fix." Replace that snprintf with aksl_strcpy(field->name, sizeof(field->name), src->name) too, following the same PASS(errctx, ...) convention. This mirrors the script-declared field path in structtype.c:376 (also aksl_strcpy, refuses rather than truncates, with limit AKBASIC_MAX_STRUCT_NAME - 1 from structtype.c:370-372). Both host.c sites should land together — fixing only the type name and leaving the field name truncating would just relocate the same collision into field lookups. 3. This changes public behaviour in two ways: a host embedder that registers a type name, or a field name, over 31 characters — which previously succeeded silently (with truncation) — will now get an error from akbasic_host_register_type. Add a short note to the function's doc comment (and CHANGELOG if one exists) calling out that host type names *and field names* are now bounded to 31 characters, matching script-declared types and fields. 4. Add a regression test (wherever host registration is already exercised, e.g. a test double registering a akbasic_HostType) covering: • a type name of exactly 31 characters (succeeds), • a type name of 32+ characters (fails with a clear error, not a truncate-and-collide), • two type names sharing a 32+ character common prefix that previously collided after truncation — confirm they now either both register successfully (if under the limit) or both fail cleanly, • the same three cases repeated for a field name instead of a type name (exactly 31 chars succeeds; 32+ chars fails cleanly; two field names on the same type sharing a 32+ character common prefix no longer collide). 5. Verify akbasic_structtype_find still behaves correctly once both type registration paths use aksl_strcpy — no change expected there, but the collision scenario described in the original comment should no longer be reachable. Likewise confirm nothing does prefix/length-sensitive lookup on field names post-fix. 6. Remove both inline comments in host.c describing this as a "latent defect rather than a settled decision" (the one at the type-name snprintf and the "same fix" note at the field-name snprintf) once the fix lands, and replace them with a short note that host and script registration now share the same 32-byte-including-NUL limit for both type names and field names. Scope: this issue is the host.c side only (both the type-name and field-name snprintf sites); no change to structtype.c's scan_names() is required, since it already does the right thing for both.
andrew added the defectstatus::grooming labels 2026-08-05 09:18:54 -04:00
Author
Owner

@tachikoma give us a plan please

@tachikoma give us a plan please
andrew added status::ready and removed status::grooming labels 2026-08-05 15:49:36 -04:00
logikoma was assigned by andrew 2026-08-05 15:49:40 -04:00
Collaborator

Beginning work on the host type/field naming-collision fix and regression coverage.

Beginning work on the host type/field naming-collision fix and regression coverage.
Collaborator

Beginning work on the naming collision fix.

Beginning work on the naming collision fix.
logikoma added status::review and removed status::ready labels 2026-08-05 18:36:49 -04:00
Collaborator

Implemented and verified the naming collision fix. Host type and field registration now use aksl_strcpy, rejecting names over 31 characters instead of truncating. Added exact-limit and overlong common-prefix regression coverage; documented the shared limit. Full build and 112/112 tests pass.

Pull request: #49

Implemented and verified the naming collision fix. Host type and field registration now use aksl_strcpy, rejecting names over 31 characters instead of truncating. Added exact-limit and overlong common-prefix regression coverage; documented the shared limit. Full build and 112/112 tests pass. Pull request: https://source.starfort.tech/andrew/akbasic/pulls/49
Sign in to join this conversation.