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

Incorrect hasher for AlignedBytes #33

Closed
V0ldek opened this issue Jun 29, 2022 · 0 comments · Fixed by #36
Closed

Incorrect hasher for AlignedBytes #33

V0ldek opened this issue Jun 29, 2022 · 0 comments · Fixed by #36
Labels
bug Something isn't working go ahead Reviewed, implementation can start good first issue Good for newcomers help wanted Extra attention is needed

Comments

@V0ldek
Copy link
Owner

V0ldek commented Jun 29, 2022

Describe the bug

Implementation of std::hash::Hash for AlignedBytes is incorrect.

MRE

use aligners::{alignment::One, AlignedBytes, AlignedSlice};
use std::{
    collections::hash_map::DefaultHasher,
    hash::{Hash, Hasher},
};

let bytes1: AlignedBytes<One> = AlignedBytes::new_zeroed(4);
let bytes2: AlignedBytes<One> = AlignedBytes::new_zeroed(4);

let mut s1 = DefaultHasher::new();
bytes1.hash(&mut s1);
let h1 = s1.finish();

let mut s2 = DefaultHasher::new();
bytes2.hash(&mut s2);
let h2 = s2.finish();

assert_eq!(h1, h2);

Expected behavior
Sequences of bytes with the same contents should have equal hashes.

Workarounds (optional)
Casting to &[u8] and then hashing works:

use aligners::{alignment::One, AlignedBytes, AlignedSlice};
use std::{
    collections::hash_map::DefaultHasher,
    hash::{Hash, Hasher},
};

let bytes1: AlignedBytes<One> = AlignedBytes::new_zeroed(4);
let bytes2: AlignedBytes<One> = AlignedBytes::new_zeroed(4);

let mut s1 = DefaultHasher::new();
let slice1: &[u8] = &bytes1;
slice1.hash(&mut s1);
let h1 = s1.finish();

let mut s2 = DefaultHasher::new();
let slice2: &[u8] = &bytes2;
slice2.hash(&mut s2);
let h2 = s2.finish();

assert_eq!(h1, h2);

Proposed solution (optional)
The hash implementation hashes the pointer to the slice. It should just delegate to [u8]'s impl of Hash.

Desktop (please complete the following information):

  • Rust version: any
  • Target triple: any
  • Features: any
  • Version: 0.0.7
@V0ldek V0ldek added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers go ahead Reviewed, implementation can start labels Jun 29, 2022
@V0ldek V0ldek assigned V0ldek and unassigned V0ldek Jun 29, 2022
V0ldek added a commit that referenced this issue Jul 8, 2022
@V0ldek V0ldek closed this as completed in #36 Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go ahead Reviewed, implementation can start good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant