Intern no crc32 #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o endian_fixes
This fixes the last of the issues with Enum on big-endian machines. The problems we caused by the assumption that 64-, 32-, 16- and 8-bit integers would align on the LSB, which is the case for little-endian machines (but not big...). In several places where reinterpret_cast<>() was used I first assigned the Enum _value_ to a temp variable of the correct size and then passed the address of that to the checksum calculation function. High cost in one sense, but really, scalar Enum variables will be rare and Arrays of Enums are handled differently. I also have dropped the overly-complex design that used a union. It eliminated some casts, but was based on the erroneous assumption regarding alignment.
One for big- and one for litte-endian machines. I made the little endian set.
…ndian ...There's still an issue, however.
D4MarshallerTest now works
This copy of the tests hould work on both little- and big-endian hosts.
This seems better - I htink computing the CRC code when interning values is not necessary. Those should only be computed when the server is going to send values.
…ksum It is only computed by the code that serializes the response - sends it to a remote client. I added baselines specifically for the intern tests on little endian machines. Will add ones for big endian next...
I dropped computing the checksum in the intern_data() method, so the baselines for the tests had to be updated.
|
When will you tackle the signed 8-bit mapping issue? Kent So, I got to thinking, while I'm fixing the big-endian crc issue, maybe this is a good time to sort out another lingering issue: that computing the checksum during intern_data() is silly - the values are only going to be used by the server again. So this branch undoes that error. You can view, comment on, or merge this pull request online at: Commit Summary
File Changes
Patch Links: — |
I’m going to try for that in the next two weeks. James
James Gallagher |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So, I got to thinking, while I'm fixing the big-endian crc issue, maybe this is a good time to sort out another lingering issue: that computing the checksum during intern_data() is silly - the values are only going to be used by the server again. So this branch undoes that error.