Skip to content

Issue #1080: Replace the original usage of string.GetHashCode() with … #1121

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Issue #1080: Replace the original usage of string.GetHashCode() with … #1121

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa56bf1
Issue #1080: Replace the original usage of string.GetHashCode() with …
May 1, 2025
d9784a8
modify hash type from int to uint
May 7, 2025
180d3f3
unchecked
May 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 54 additions & 1 deletion src/NetMQ/Core/Transports/Ipc/IpcAddress.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void Resolve(string name, bool ip4Only)
{
m_name = name;

int hash = name.GetHashCode();
int hash = GetMurmurHash(name);
if (hash < 0)
hash = -hash;
hash = hash%55536;
Expand All @@ -48,6 +48,59 @@ public void Resolve(string name, bool ip4Only)
this.Address = new IPEndPoint(IPAddress.Loopback, hash);
}

/// <summary>
/// Calculate hash values using the MurmurHash algorithm
/// </summary>
/// <param name="name">input</param>
/// <returns>hash value</returns>
private static int GetMurmurHash(string name)
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if we could move this into a util class, like Strings, so it can be reused more easily in future.

Copy link
Member

Choose a reason for hiding this comment

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

The implementation could be wrapped in unchecked { ... }.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your suggestion, I have modified the code. 😊

Copy link
Member

Choose a reason for hiding this comment

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

It can still throw if someone compiles in checked mode. The casts between int/uint need to be unchecked.

Copy link
Author

Choose a reason for hiding this comment

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

thanks for your reply, The result of hash % 55536 ranges from 0 to 55,535, which is a safe range since it does not exceed the positive integer limit. Therefore, (int)(hash % 55536) will not cause an overflow.

Copy link
Member

Choose a reason for hiding this comment

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

I believe there are still two places that would fail checked arithmetic.

hash ^= (uint)(name[currentIndex + 2] << 16);

If the char has the high bit set, the result is a negative integer. Casting to uint will throw I think.

Similarly:

                uint k = (uint)(name[currentIndex] |
                                (name[currentIndex + 1] << 8) |
                                (name[currentIndex + 2] << 16) |
                                (name[currentIndex + 3] << 24));

Here's an example:

https://sharplab.io/#v2:EYLgHgbALANALiATgVwHYB8DGALAppga1wBMBYAKAG8KACOgAQEYA6JgTgAoPkBLVOAJQcARIB4NwCD7wgNoAGALo0APIpqMIAgQG4KAXyA=

The code seems to assume that char is int8 but they're actually int16.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your advice, I modify the code.

{
const uint seed = 0xc58f1a7b;
const uint m = 0x5bd1e995;
const int r = 24;

uint hash = seed ^ (uint)name.Length;
int length = name.Length;
int currentIndex = 0;

while (length >= 4)
{
uint k = (uint)(name[currentIndex] |
(name[currentIndex + 1] << 8) |
(name[currentIndex + 2] << 16) |
(name[currentIndex + 3] << 24));

k *= m;
k ^= k >> r;
k *= m;

hash *= m;
hash ^= k;

currentIndex += 4;
length -= 4;
}

switch (length)
{
case 3:
hash ^= (uint)(name[currentIndex + 2] << 16);
goto case 2;
case 2:
hash ^= (uint)(name[currentIndex + 1] << 8);
goto case 1;
case 1:
hash ^= name[currentIndex];
hash *= m;
break;
}

hash ^= hash >> 13;
hash *= m;
hash ^= hash >> 15;
return (int)hash;
}

[DisallowNull]
public IPEndPoint? Address { get; private set; }

Expand Down
Loading