Skip to content

fix: TEST_NUM_from_record reads full 4-byte U*4 (closes #76) - #79

Open
seam0814 wants to merge 1 commit into
Semi-ATE:mainfrom
seam0814:fix-test-num-from-record-slice
Open

fix: TEST_NUM_from_record reads full 4-byte U*4 (closes #76)#79
seam0814 wants to merge 1 commit into
Semi-ATE:mainfrom
seam0814:fix-test-num-from-record-slice

Conversation

@seam0814

Copy link
Copy Markdown

Summary

TEST_NUM_from_record extracts the test number from a PTR, MPR or
FTR record. TEST_NUM is a U*4 field at byte offset 4..7
(inclusive), so the slice fed into struct.unpack(\"...I\", ...) must
be record[4:8], not record[4:7].

The 3-byte slice raises struct.error: unpack requires a buffer of 4 bytes on every invocation, so the function is effectively dead on
arrival for the only record types it is supposed to handle.

Closes #76.

Change

Semi_ATE/STDF/utils.pyTEST_NUM_from_record:

-        TEST_NUM = struct.unpack(\"%sI\" % endian, record[4:7])
+        TEST_NUM = int(struct.unpack(\"%sI\" % endian, record[4:8])[0])
  • record[4:7]record[4:8] (the actual bug)
  • wrap the unpack result with int(...)[0] so the success path
    returns the same type as the sentinel (-1, an int). Without
    this, callers have to branch on tuple vs int — almost
    certainly unintended given the sentinel is already an int.
  • docstring offset table updated (4:74:8)

Test

Three new parametric pytest cases in tests/test_utils.py:

  1. Round-trip with TEST_NUM = 0x12345678 — value is larger than
    2**24, so any MSB-byte truncation would surface. Runs under both
    little- and big-endian.
  2. Sentinel path — non-test record (e.g. FAR) returns
    int(-1), not a tuple.
  3. All three test record subtypesPTR (15,10), MPR (15,15), and FTR (15,20) all read TEST_NUM correctly.

Full suite: 31 passed, 4 skipped, 0 failed.

```
$ python -m pytest tests/test_utils.py -v -k TEST_NUM
tests/test_utils.py::test_TEST_NUM_from_record_roundtrip[<] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_roundtrip[>] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_returns_int_for_non_test_record PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[10] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[15] PASSED
tests/test_utils.py::test_TEST_NUM_from_record_handles_all_test_subtypes[20] PASSED
```

Scope

Pure bug fix, no public API change beyond the (almost certainly
unintended) tuple→int return-type tightening. No other call sites of
TEST_NUM_from_record exist in the repository.

`TEST_NUM` is a U*4 field at byte offset 4..7 (inclusive) in PTR/MPR/FTR
records, so the slice into `struct.unpack` must be `record[4:8]`, not
`record[4:7]`. The 3-byte slice raised `struct.error: unpack requires a
buffer of 4 bytes` on every call, making the function effectively dead
on arrival.

Also:
- wrap the unpack result with `int(...)[0]` so the success path matches
  the sentinel return value (`-1`, an int) and callers don't have to
  branch on tuple vs int
- fix the docstring offset table (4:7 → 4:8)
- add three parametric pytest cases:
  - round-trip with TEST_NUM = 0x12345678 (> 2**24, exposes any
    MSB-byte truncation) under both little- and big-endian
  - sentinel path returns int(-1), not tuple, for non-test records
  - all three test record subtypes (PTR 15/10, MPR 15/15, FTR 15/20)
    read TEST_NUM correctly

Full suite: 31 passed, 4 skipped.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@seam0814
seam0814 requested review from nerohmot and seimit as code owners June 21, 2026 06:58
@seam0814

Copy link
Copy Markdown
Author

Gentle nudge — small parser fix reading full 4-byte U*4 for TEST_NUM (closes #76). Happy to rebase against master or add a regression test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TEST_NUM_from_record has wrong range [4:7] in unpack, int will need 4 bytes [4:8]

1 participant