Skip to content

murmur2: fix undefined behaviour#42

Merged
MabezDev merged 1 commit into
rust-embedded-community:mainfrom
newAM:murmur3-fix-ub
Apr 28, 2026
Merged

murmur2: fix undefined behaviour#42
MabezDev merged 1 commit into
rust-embedded-community:mainfrom
newAM:murmur3-fix-ub

Conversation

@newAM

@newAM newAM commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

The Buffer struct held a MaybeUninit<[u8; 4]> which was written to byte-by-byte via assume_init_mut. The problem is that assume_init_mut requires the entire value is fully initialized.

This fix changes Buffer.bytes to [MaybeUninit<u8>; 4], where each byte is its own MaybeUninit. Writes go through MaybeUninit::write() on individual elements which has no initialization requirement.

The Buffer struct held a MaybeUninit<[u8; 4]> which was written to
byte-by-byte via assume_init_mut. The problem is that assume_init_mut
requires the entire value is fully initialized.

This fix changes Buffer.bytes to [MaybeUninit<u8>; 4], where each byte
is its own MaybeUninit. Writes go through MaybeUninit::write() on
individual elements which has no initialization requirement.
@newAM

newAM commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

CC @eldruin

@Manishearth

Copy link
Copy Markdown
Contributor

If you're doing this can you add safety comments on every assume_init that explains why it is known to be initialized at that point?

@Manishearth Manishearth mentioned this pull request Apr 27, 2026

@Manishearth Manishearth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Went ahead and did it myself in #43. But this PR looks good.

@MabezDev MabezDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@MabezDev MabezDev merged commit f1816a7 into rust-embedded-community:main Apr 28, 2026
5 checks passed
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.

3 participants