Skip to content

fix: handle optional DA checksum field in LCOV parser#1500

Closed
robertomlsoares wants to merge 1 commit into
mozilla:masterfrom
robertomlsoares:da-checksum-fix
Closed

fix: handle optional DA checksum field in LCOV parser#1500
robertomlsoares wants to merge 1 commit into
mozilla:masterfrom
robertomlsoares:da-checksum-fix

Conversation

@robertomlsoares
Copy link
Copy Markdown

The LCOV format allows an optional checksum field on DA lines: DA:line_number,execution_count[,checksum]

When a checksum starts with e, the byte-level parser mistakes it for an end_of_record marker, since it only checks the first byte. This causes a panic at parser.rs:194 (unwrap on None).

The fix validates the full end_of_record string instead of matching on just the e byte. Lines starting with e that don't match end_of_record are skipped.

The LCOV format allows an optional checksum field on DA lines:
DA:line_number,execution_count[,checksum]

When a checksum starts with 'e', the byte-level parser mistakes it
for an "end_of_record" marker, since it only checks the first byte.
This causes a panic at parser.rs:194 (unwrap on None).

The fix validates the full "end_of_record" string instead of
matching on just the 'e' byte. Lines starting with 'e' that don't
match "end_of_record" are skipped.
@robertomlsoares
Copy link
Copy Markdown
Author

Looks like the linter is failing on pre-existing code.

@robertomlsoares
Copy link
Copy Markdown
Author

I have tried this fix in our systems, and while it did work for checksums starting with e, it made me realise this issue is much more widespread. It affects all checksums that have starting characters equal to some LCOV "keywords" such as FN, DA, SF, etc. The appropriate fix is much more complex than what this PR is currently doing, so I will close it and leave the problem documented here #1501.

@robertomlsoares robertomlsoares deleted the da-checksum-fix branch May 21, 2026 13:37
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.

1 participant