Skip to content

Simplify code for parsing comments#752

Open
jeltz wants to merge 1 commit into
percona:mainfrom
jeltz:simplify-comment-parsing
Open

Simplify code for parsing comments#752
jeltz wants to merge 1 commit into
percona:mainfrom
jeltz:simplify-comment-parsing

Conversation

@jeltz

@jeltz jeltz commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

The code was hard to read for no obvious gain so this attempts to make the code read more like one would expect. Also remove a redundant comment.

PG-0

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.84211% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.68%. Comparing base (86edb37) to head (6fec9fd).

Files with missing lines Patch % Lines
src/pg_stat_monitor.c 36.84% 6 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #752      +/-   ##
==========================================
- Coverage   88.33%   87.68%   -0.65%     
==========================================
  Files           3        3              
  Lines        1329     1332       +3     
  Branches      201      202       +1     
==========================================
- Hits         1174     1168       -6     
- Misses         72       79       +7     
- Partials       83       85       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dutow dutow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a possible over-read in this code. While the pr only aims for simplification, should we also fix that too, if we are already changing this area of code?

SELECT 1 /*tail*/ or SELECT 1 /*oops

inner while loop consumes */ and points to the null. End of foor loop increases pointer again. *q_iter used by the for loop is now the character after null.

The unterminated comment is similar, the while loop already ends with q_iter pointing to the null byte.

@jeltz jeltz force-pushed the simplify-comment-parsing branch 2 times, most recently from a21642c to 6f7faa4 Compare July 5, 2026 10:08
@jeltz

jeltz commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@dutow Does this fix look correct for the over-read?

The code was hard to read for no obvious gain so this attempts to make
the code read more like one would expect. Also remove a redundant
comment.

Also fixes a potential off by one read.
@jeltz jeltz force-pushed the simplify-comment-parsing branch from 6f7faa4 to 6fec9fd Compare July 5, 2026 10:10
@jeltz jeltz requested a review from dutow July 5, 2026 10:15

@dutow dutow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now.

We should add more coverage to this (the cases I already mentioned for asan + maybe something like SELECT 1 /*a*//*b*/, but that can also be a separate PR.

@dutow

dutow commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Another question: since this now also includes a bugfix, it should have a proper ticket?

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.

2 participants