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

fix 'mmh3_hash' and 'mmh3_hash_from_buffer' result handling on 32bit … #40

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

dshein-alt
Copy link
Contributor

@dshein-alt dshein-alt commented Nov 9, 2022

linux: i586, armh

  • added conditional 'mmh3_hash' and 'mmh3_hash_from_buffer' functions result handling depending on target platform architecture

Current 32bit hash functions result conversion method fails on 32 bit architectures (confirmed on i586 and armh platforms).

__________________________________________ test_hash_unsigned __________________________________________

    def test_hash_unsigned():
>       assert mmh3.hash("foo", signed=False) == 4138058784
E       AssertionError: assert -156908512 == 4138058784
E        +  where -156908512 = <built-in function hash>('foo', signed=False)
E        +    where <built-in function hash> = mmh3.hash

test_mmh3.py:74: AssertionError
________________________________________ test_hash_from_buffer _________________________________________

    def test_hash_from_buffer():
        mview = memoryview("foo".encode("utf8"))
        assert mmh3.hash_from_buffer(mview) == -156908512
>       assert mmh3.hash_from_buffer(mview, signed=False) == 4138058784
E       assert -156908512 == 4138058784
E        +  where -156908512 = <built-in function hash_from_buffer>(<memory at 0xf71d82c8>, signed=False)
E        +    where <built-in function hash_from_buffer> = mmh3.hash_from_buffer

test_mmh3.py:181: AssertionError
======================================= short test summary info ========================================
FAILED test_mmh3.py::test_hash_unsigned - AssertionError: assert -156908512 == 4138058784
FAILED test_mmh3.py::test_hash_from_buffer - assert -156908512 == 4138058784

On 64 bit architectures (confirmed on x86_64, aarch64 and ppc64le platforms) applied conversion methods works fine and all tests passed.

Preprocessor conditions added to modify code when built on 32 bit platforms using predefined __LONG_WIDTH__ symbol available with GCC and Clang compilers.r conditions to modify code when built on 32 bit platforms using predefined __LONG_WIDTH__ symbol available with GCC and Clang compilers.

dshein-alt and others added 2 commits November 9, 2022 08:56
…platforms

- added conditional 'mmh3_hash'	and 'mmh3_hash_from_buffer' functions result
handling depending on target platform architecture
@hajimes hajimes merged commit 64e1b77 into hajimes:master Mar 24, 2023
@hajimes
Copy link
Owner

hajimes commented Mar 24, 2023

Sorry for the delay, but now I merged your commit (with some modifications to prevent building failures on macos).

I am really thankful to you for providing detailed explanations so that the reader can easily follow problems and improvements.

@dshein-alt
Copy link
Contributor Author

Glad to help, mate! ))

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