magiblot
commented
7 months ago Hi Mark!
Thank you very much for submitting this PR and excuse me for the late response.
I grabbed the following changes from it:
- The macro name (
TV_BIG_ENDIAN). - The CMake endianess auto-detection (since I didn't know the
TestBigEndianscript existed, I was going to usecheck_cxx_source_compiles, but I guess it's better to use the CMake-provided method).
As for the rest, it turns out it is not always necessary to reverse the bytes when copying. For example, TCellChar::moveInt is only used for copying an integer which had been previously bit-casted from a byte array, such as the result of CpTranslator::toPackedUtf8. In this case the comment was not clarifying enough.
In fast_btoa I found a way to keep the optimization without using bit-casting, so I used that instead.
If you are interested, commit f933af5e18c2bc7b3ead2778447d5b1b171d1df4 contains all the changes.
MookThompson
The fixes are broadly in 3 areas: o use of memcpy to copy between char and integer types. In my PR, I created a 'tvintmemcpy', which #defines to the normal memcpy on LE platforms (so no runtime changes), but does a crude byte-reversed memcpy on BE ones. I then went through and changed any memcpy calls where one param is an integer pointer and the other is a char* to use this new tvintmemcpy. This issue affects TermColor::operator= and TermColor::operator uint32_t, CpTranslator::toPackedUtf8, utf32To8, TCellChar::moveInt, and the struct colorconv_r::colorconv_r(TermColor aColor...) constructor. string_as_int has a similar issue, but I've had to address it slightly differently. I've noticed that platform/far2l.cpp seems to have 12 memcpy calls which look like they would need to also be replaced by tvintmemcpy, but I'm unable to test this so haven't made these changes.
o the optimisation of fast_btoa, as the comment says, is only suitable for LE platforms as the btoa_lut_elem_t is a compound struct (3 chars and a uint8). In my PR, I've provided a non-optimised version under the BE #define, which also (probably unnecessarily) null-terminates the destination buffer.
o The struct KeyDownEvent relies on a union of ushort and CharScanType types, but the order of the elements of the CharScanType are reversed on BE platforms. To minimise the diff, I've just reversed the order of the charCode and scanCode members of the struct CharScanType under the TV_BIG_ENDIAN define.