Skip to content

fix(pag): Incorrect calculation of last used page#8772

Merged
dyemanov merged 3 commits intoFirebirdSQL:masterfrom
TreeHunter9:master-last-used-page
Mar 21, 2026
Merged

fix(pag): Incorrect calculation of last used page#8772
dyemanov merged 3 commits intoFirebirdSQL:masterfrom
TreeHunter9:master-last-used-page

Conversation

@TreeHunter9
Copy link
Copy Markdown
Contributor

@TreeHunter9 TreeHunter9 commented Oct 13, 2025 •
edited
Loading

If pip_used has its maximum possible value, we can read over the page buffer.
This can lead to incorrect validation of SCN pages when requesting the last used page to calculate the number of SCNs.

v5 and v4 are also affected.

@dyemanov dyemanov requested a review from hvlad March 20, 2026 04:37
hvlad
hvlad requested changes Mar 20, 2026
View reviewed changes
Comment thread src/jrd/pag.cpp Outdated
@dyemanov dyemanov merged commit d5bf064 into FirebirdSQL:master Mar 21, 2026
23 checks passed
dyemanov pushed a commit that referenced this pull request Mar 21, 2026
@TreeHunter9 @dyemanov
fix(pag): Incorrect calculation of last used page (#8772)
b91456b 
* fix(pag): Incorrect calculation of last used page

* Revert "fix(pag): Incorrect calculation of last used page"

This reverts commit 91a29a8.

* fix(pag): Incorrect calculation of last used page

---------

Co-authored-by: Artyom Ivanov <[email protected]>
@TreeHunter9
Copy link
Copy Markdown
Contributor Author

TreeHunter9 commented Mar 23, 2026

It looks like the patch is incorrect when we are working with the first PIP. Because the first PIP is interpreted as 0 page, the last_bit + 1 logic results in an unneeded offset being added.

I will prepare a patch with the fix.

@hvlad
Copy link
Copy Markdown
Member

hvlad commented Mar 23, 2026

last_bit is the relative number of the last used page. When it reside on first PIP, it become absolute number.
What is wrong ?

@TreeHunter9
Copy link
Copy Markdown
Contributor Author

TreeHunter9 commented Mar 23, 2026

When it reside on first PIP, it become absolute number.

Yes, but we add 1 to this absolute number, so the result page number is wrong.

@hvlad
Copy link
Copy Markdown
Member

hvlad commented Mar 23, 2026

You right, I (wrongly) considered that it should return count of used pages.

@aafemt
Copy link
Copy Markdown
Contributor

aafemt commented Mar 23, 2026 •
edited
Loading

Still it is not clear why page zero is different from others and what happen with the gap between the last bit on this page and first bit on the next.

@TreeHunter9
Copy link
Copy Markdown
Contributor Author

TreeHunter9 commented Mar 23, 2026

Still it is not clear why page zero is different from others and what happen with the gap between the last bit on this page and first bit on the next.

I'm not quite getting your question, but Vlad explained how last_bit works above.

@TreeHunter9 TreeHunter9 mentioned this pull request Mar 23, 2026
Merged
@aafemt
Copy link
Copy Markdown
Contributor

aafemt commented Mar 23, 2026

Ugh, yes. What was missed in this explanation was that pipLast is not just pagesPerPip*pageNumber which makes whole calculation unnecessarily complicated.

@mrotteveel mrotteveel added the rlsnotes60: no Intentionally not added to the Firebird 6.0 release notes. label Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hvlad hvlad hvlad approved these changes

Assignees

@TreeHunter9 TreeHunter9

Labels

fix-version: 5.0.4 fix-version: 6.0 Alpha 1 rlsnotes60: no Intentionally not added to the Firebird 6.0 release notes. type: bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TreeHunter9 @hvlad @aafemt @mrotteveel @dyemanov