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

Should not care about endianness #55

Open
fsateler opened this issue Feb 25, 2016 · 6 comments
Open

Should not care about endianness #55

fsateler opened this issue Feb 25, 2016 · 6 comments
Milestone

Comments

@fsateler
Copy link
Contributor

Most software does not need to care about native endianness, it just needs to convert to/from defined endianness to native when writing/reading, and that does not require actually knowing the native endianness.

From Rob Pike's blog:

If the data stream encodes values with byte order B, then the algorithm to decode the value on computer with byte order C should be about B, not about the relationship between B and C.

That same post also heplfully describes how to read little or big endian 32 bit numbers:

LE: i = (data[0]<<0) | (data[1]<<8) | (data[2]<<16) | (data[3]<<24);
BE: i = (data[3]<<0) | (data[2]<<8) | (data[1]<<16) | (data[0]<<24);

However, it doesn't provide examples on how to write the numbers (which is likely the reverse of the above, but I need to verify first).

Not critical, but filing as an issue so that I don't forget.

@fsateler
Copy link
Contributor Author

Here is a very helpful public domain header file: https://gist.github.com/panzi/6856583

@arielelkin
Copy link
Member

@fsateler what can be specifically done about this?

@fsateler
Copy link
Contributor Author

fsateler commented Aug 8, 2016

@arielelkin I tried to do a patch for this but became quickly confused. Currently, stk has plenty of code of the following form:

int value = read_from_somewhere();
#ifdef __LITTLE_ENDIAN__
swap32((unsigned char *)&value);
#endif

Conditions vary depending on whether the file being read is little or big endian. The replacement using the linked gist header file, looks like this:

int value = read_from_somewhere();
value = be32toh(value);

The core point is that knowledge of the native endianness is not needed (in the linked file, only Windows needs such knowledge, ideally one would find another one that does not require even that): all that is needed is to know what format is being read/written from/to.

Patch is not necessarily difficult, but requires a lot of care to not invert any given conversion.

The advantage of not requiring this knowledge, is that every downstream project that allows bundling STK code can now drop the endian checks, which they probably do not need.

@arielelkin
Copy link
Member

@garyscavone any thoughts?

@garyscavone
Copy link
Contributor

If there are standard functions available on all supported platforms and OS’es to convert between host and big or little endian, then it would be nice to make this change. But if they only exist with the most modern versions of C++ and/or are not standard on all platforms and compilers (ex. Windows), then it should not be done.

On Aug 8, 2016, at 9:40 AM, Ariel Elkin [email protected] wrote:

@garyscavone https://github.com/garyscavone any thoughts?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #55 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AFOBpRB9edObkOHoKt7zjVYlrurCnPeoks5qdzGwgaJpZM4HixWF.

{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/thestk/stk","title":"thestk/stk","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/modules/aws/aws-bg.jpg","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/thestk/stk"}},"updates":{"snippets":[{"icon":"PERSON","message":"@arielelkin in #55: @garyscavone any thoughts?"}],"action":{"name":"View Issue","url":"https://github.com/thestk/stk/issues/55#issuecomment-238239911"}}}

@fsateler
Copy link
Contributor Author

@garyscavone Agreed. I think what is needed is a full set of conversions like the ones I quoted in the first post

@radarsat1 radarsat1 added this to the 4.6.0 milestone Aug 16, 2017
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

No branches or pull requests

4 participants