Skip to content

Remove erroneous cast to size_t in BID_HASH_FUNC #10

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

Merged
merged 1 commit into from
May 24, 2021

Conversation

funny-falcon
Copy link
Contributor

It were mistakenly remain after change of BID_HASH_FUNC meaning

@ololobus ololobus self-requested a review May 24, 2021 09:08
@@ -86,7 +86,7 @@ typedef PtrackMapHdr * PtrackMap;
/* Block address 'bid' to hash. To get slot position in map should be divided
* with '% PtrackContentNblocks' */
#define BID_HASH_FUNC(bid) \
(size_t)(DatumGetUInt64(hash_any_extended((unsigned char *)&bid, sizeof(bid), 0)))
(DatumGetUInt64(hash_any_extended((unsigned char *)&bid, sizeof(bid), 0)))
Copy link
Contributor

@ololobus ololobus May 24, 2021

Choose a reason for hiding this comment

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

Yeah, this size_t seems to be not correct, since it is 32 bit wide on 32 bit systems, which is not what we want here. However, next we assign this value to hash variable, which again has a size_t type, so this change alone does not has much sense, doesn't it? Probably we should change all appearances of size_t used for hash map reference to uint64 for portability?

Copy link
Contributor Author

@funny-falcon funny-falcon May 24, 2021

Choose a reason for hiding this comment

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

Yep, I recognize hash variable as well. I've just push-forced branch.

It were mistakenly remain after change of BID_HASH_FUNC meaning
Copy link
Contributor

@ololobus ololobus left a comment

Choose a reason for hiding this comment

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

LGTM now

@ololobus ololobus merged commit 8d42b6b into postgrespro:master May 24, 2021
@funny-falcon funny-falcon deleted the patch-1 branch May 24, 2021 09:47
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