Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verifychecksum: In case of checksum mismatch, expected and actual are reversed in error message #48

Closed
mbanck opened this issue Jan 23, 2018 · 8 comments

Comments

@mbanck
Copy link

mbanck commented Jan 23, 2018

No description provided.

@mbanck
Copy link
Author

mbanck commented Jan 23, 2018

patch for this:

--- verifychecksum.c.orig       2018-01-23 14:12:23.000000000 +0100
+++ verifychecksum.c.new        2018-01-23 14:35:30.000000000 +0100
@@ -23,7 +23,7 @@
   if (phdr->pd_checksum != checksum)
     {
       printf("%s: blkno %d, expected %x, found %x\n",
-            filepath, blkno, checksum, phdr->pd_checksum);
+            filepath, blkno, phdr->pd_checksum, checksum);
       return FALSE;
     }

@snaga
Copy link
Contributor

snaga commented Jan 27, 2018

In this case, phdr->pd_checksum is a stored value in the page header and checksum is a calculated (on-the-fly) value from the page content. Here, the latter one should be treated as the correct checksum.

So, I handle phdr->pd_checksum as found, and checksum as expected.

Doesn't it make sense?

@mbanck
Copy link
Author

mbanck commented Jan 27, 2018

I was looking at it from Postgres' perspective: it has stored phdr->pd_checksum in the page header, so it expects this checksum to be there. But actually, we found checksum.

@snaga
Copy link
Contributor

snaga commented Feb 1, 2018

I see.
Need more time to look for more appropriate words.

@snaga
Copy link
Contributor

snaga commented Feb 1, 2018

How about "page header" for phdr->pd_checksum and "calculated" as checksum?
Does it make sense?

@mbanck
Copy link
Author

mbanck commented Feb 1, 2018

Sounds good to me, yeah.

snaga added a commit that referenced this issue Feb 2, 2018
@snaga
Copy link
Contributor

snaga commented Feb 2, 2018

Committed. 9ef3e48

snaga added a commit that referenced this issue Feb 2, 2018
@snaga
Copy link
Contributor

snaga commented Feb 3, 2018

Merged. #60

@snaga snaga closed this as completed Feb 3, 2018
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

No branches or pull requests

2 participants