fix(index): conditional FOR ignored a logical-field condition

[Admnwk]

Problem

A conditional index whose FOR clause is a bare logical field — e.g.

INDEX ON AGE TO agetag FOR ACTIVE

indexed every record instead of only the rows where ACTIVE is true.
Against native DBFCDX (the oracle) the same expression yields a smaller key
count; OpenADS CDX returned the full table count.

Root cause

In engine/index_expr.cpp, parse_atom evaluated a bare field reference by
falling back to a truthiness test on its string value (!s.empty()). A logical
field's as_string is "T" or "F" — both non-empty — so the predicate
was truthy for every row, and the conditional FOR filter matched everything.

Fix

Carry a bare Logical field as a numeric 1/0 (from as_bool) instead of a
string, so the FOR predicate evaluates to the field's actual boolean value.
8 lines, single file (src/engine/index_expr.cpp); no ABI/wire changes.

Tests

• tests/unit/abi_qa_repro_test.cpp — engine-level repro built through the ABI
  on a real DBF/CDX table: case A (INDEX ON AGE FOR ACTIVE honours the
  condition) and case C (numeric ordScope range returns rows). Native DBFCDX
  is the oracle. RED before the fix, GREEN after; full suite passes.
• tools/qa-diff/ — a small Harbour differential harness that runs the same
  xBase operations against native DBFCDX/NTX and OpenADS and diffs the results,
  which is how the divergence was found. Included for future regression hunting.

---

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request fixes an issue where bare logical fields in conditional index expressions (e.g., FOR ACTIVE) were incorrectly evaluated using string truthiness instead of their boolean value. It also introduces a differential QA harness tool (qa-diff) in Harbour to compare OpenADS against native DBFCDX/DBFNTX, and adds corresponding unit tests. The review feedback highlights three issues in the new unit tests: a potential out-of-bounds read when copying truncated strings, potential test flakiness from leftover index files, and a potential buffer overflow when copying directory paths into a fixed-size buffer.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

If name is longer than 63 characters, std::strncpy will truncate the copied string in v to 63 characters (leaving v[63] as \0). However, std::strlen(name) is still used as the length parameter for AdsSetString. This causes AdsSetString to read past the null-terminator of v, leading to a potential out-of-bounds read (buffer over-read) on the stack.

To prevent this, use the length of the actually copied string in v instead of the original name length.

Suggested change

	UNSIGNED8 v[64]{};
	std::strncpy(reinterpret_cast<char*>(v), name, sizeof(v) - 1);
	REQUIRE(AdsSetString(hT, fN, v,
	static_cast<UNSIGNED32>(std::strlen(name))) == AE_SUCCESS);
	UNSIGNED8 v[64]{};
	std::strncpy(reinterpret_cast<char*>(v), name, sizeof(v) - 1);
	REQUIRE(AdsSetString(hT, fN, v,
	static_cast<UNSIGNED32>(std::strlen(reinterpret_cast<char*>(v)))) == AE_SUCCESS);

[gemini-code-assist]

The open_fixture function only removes the .dbf file (dir / tbl), but does not clean up any associated index files (like .cdx or .fpt files) that might have been created in the same directory during previous test runs. This can lead to test flakiness or unexpected behavior if stale index files are left behind.

Using std::filesystem::remove_all on the directory before recreating it ensures a completely clean and isolated test environment.

Suggested change

	ADSHANDLE open_fixture(const fs::path& dir, const char* tbl) {
	std::error_code ec;
	fs::create_directories(dir, ec);
	fs::remove(dir / tbl, ec);
	ADSHANDLE open_fixture(const fs::path& dir, const char* tbl) {
	std::error_code ec;
	fs::remove_all(dir, ec);
	fs::create_directories(dir, ec);

[gemini-code-assist]

Using std::memcpy to copy dir.string() into the fixed-size buffer srv[260] without checking the size of the path can lead to a buffer overflow if the temporary directory path is extremely long.

Adding a REQUIRE assertion to ensure the path fits within the buffer prevents any potential buffer overflow.

Suggested change

	UNSIGNED8 srv[260]{};
	std::memcpy(srv, dir.string().c_str(), dir.string().size());
	UNSIGNED8 srv[260]{};
	REQUIRE(dir.string().size() < sizeof(srv));
	std::memcpy(srv, dir.string().c_str(), dir.string().size());