Skip to content

cleanup(coverage): two line counters for the same buffer disagree by one on a trailing newline #1967

Description

@CaptainMittens

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_t orig_lines = 1;
for (int ci = 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.

Related: #963, #1941.

Activity

  1. added
    bugSomething isn't working
    priority/highNeeds near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.
    parsing/qualityGraph extraction bugs, false positives, missing edges
    on Sep 5, 2026
  2. DeusData commented on Sep 25, 2026

    @DeusData
    Owner

    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!

  3. DeusData commented on Oct 7, 2026

    @DeusData
    Owner

    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!

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingparsing/qualityGraph extraction bugs, false positives, missing edgespriority/highNeeds near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions