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.

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

Jump to bottom

Binary endianness safety #41

Closed
moien007 opened this issue Mar 9, 2020 · 4 comments
Closed

Binary endianness safety #41

moien007 opened this issue Mar 9, 2020 · 4 comments
Assignees
@daviddesmet
Labels
enhancement New feature or request

Comments

@moien007
Copy link
Contributor

moien007 commented Mar 9, 2020

Hi,

The class ByteIntegerConverter have methods like LoadLittleEndian32 and StoreLittleEndian32 which is not checking for system endianness (which is not guaranteed to be little-endian in .NET). In the old .NET days, we were checking System.BitConverter.IsLittleEndian field to finding out system endianness and reversing the bytes to achieve the endianness that we were looking for, but in this modern .NET age we should consider using System.Buffers.BinaryPrimitives class (my benchmarks showed me it is significantly faster than bitwise operations like << and || since it takes advantage of CPU instructions like rotl and rotr on X86 machines).
@daviddesmet daviddesmet added the enhancement New feature or request label Mar 9, 2020
@daviddesmet
Copy link
Owner

Hello again 🙂

Seems you already made some local changes... would you kindly help the project with a PR of your changes? It will be a great addition to the library.

@moien007
Copy link
Contributor Author

Hello again 🙂

Seems you already made some local changes... would you kindly help the project with a PR of your changes? It will be a great addition to the library.

Sure, I will address #40 in my PR too.

@daviddesmet
Copy link
Owner

Sure, I will address #40 in my PR too.

Sounds great! Looking forward to it 🙂

@moien007 moien007 mentioned this issue Mar 10, 2020
Merged
@daviddesmet daviddesmet self-assigned this Mar 10, 2020
@daviddesmet
Copy link
Owner

daviddesmet commented Mar 18, 2020 •
edited

This was closed by fd6ae39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@daviddesmet daviddesmet

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@daviddesmet @moien007