You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
cleanup(coverage): two line counters for the same buffer disagree by one on a trailing newline #1967
Two places in internal/cbm/cbm.c count the lines of the same source buffer and disagree by one on any file that ends with a newline — which is nearly every file.
cbm_count_lines (around internal/cbm/cbm.c:1372) ignores a final newline:
if (src[i] =='\n'&&i+1<src_len) {
n++;
}
The inline counter that builds orig_lines for the preprocessed-line map (around internal/cbm/cbm.c:1772) counts every newline:
uint32_torig_lines=1;
for (intci=0; ci<source_len; ci++) {
if (source[ci] =='\n') {
orig_lines++;
}
}
Nothing is broken today
The two feed different consumers. cbm_count_lines decides the parse_unusable 80% threshold, where being one line generous costs nothing. orig_lines sizes an array and bounds a loop, where being one line generous is the safe direction. Raised by review on #1941 as a maintenance risk, not a live defect.
Why it is worth closing
A file whose report claims to say honestly which lines are missing should not hold two different answers to "how many lines does this file have". The next reader who reaches for either counter has no way to know which convention they picked up.
Suggested fix
One helper, one convention, both callers on it — with the chosen convention written down in a comment beside the helper, since either answer is defensible and only the disagreement is not.
Thank you so much for writing this up, and for being clear that it was a maintenance risk rather than a live defect. That made it easy to fix calmly. You were right, and there turned out to be a third counter: cbm_line_offsets, used by the recovery-gap walker, also counted every newline.
In #2336 all three go through one helper, cbm_source_line_count(), with the convention written next to it: a newline ends a line and does not start a new one (so a\nb and a\nb\n both have 2 lines), and an empty buffer counts as 1 line. As you predicted, no report output changes, but the coverage report now has only one answer to "how many lines does this file have", and a unit test pins the convention. Thanks again for the careful eye after #1941!
This has now landed on main in 731f624 (#2336) and will ship in the next release. Thank you again, @CaptainMittens, for spotting it before it could ever bite!
Activity
DeusData commentedon Sep 25, 2026
Thank you so much for writing this up, and for being clear that it was a maintenance risk rather than a live defect. That made it easy to fix calmly. You were right, and there turned out to be a third counter:
cbm_line_offsets, used by the recovery-gap walker, also counted every newline.In #2336 all three go through one helper,
cbm_source_line_count(), with the convention written next to it: a newline ends a line and does not start a new one (soa\nbanda\nb\nboth have 2 lines), and an empty buffer counts as 1 line. As you predicted, no report output changes, but the coverage report now has only one answer to "how many lines does this file have", and a unit test pins the convention. Thanks again for the careful eye after #1941!fix(coverage): one line counter for a source buffer (#1967)
fix(coverage): one line counter for a source buffer (#1967)
fix(coverage): one line counter for a source buffer (#1967)
Merge pull request #2336 from DeusData/fix/issue-1967
DeusData commentedon Oct 7, 2026
This has now landed on main in 731f624 (#2336) and will ship in the next release. Thank you again, @CaptainMittens, for spotting it before it could ever bite!
Merge main into fix/issue-2078 for the release batch