Fix endianness and alignment assumptions in pnNpCommon IO code#1850
Fix endianness and alignment assumptions in pnNpCommon IO code#1850dgelessus wants to merge 11 commits intoH-uru:masterfrom
pnNpCommon IO code#1850Conversation
dpogue
left a comment
dpogue
left a comment
There was a problem hiding this comment.
This is definitely nicer than the hacky string copy stuff I had to do in plNglFile
Sources/Plasma/CoreLib/hsEndian.h
Outdated
|
|
||
| #include "HeadSpin.h" | ||
|
|
||
| #include <string_theory/string> |
There was a problem hiding this comment.
Could we use forward declarations instead?
There was a problem hiding this comment.
Probably. I was too lazy to type out the template declaration needed for ST::char16_buffer :D
There was a problem hiding this comment.
Changing the return type of hsSTStringToUTF16LE as suggested above would also resolve this ;)
Sources/Plasma/CoreLib/hsEndian.cpp
Outdated
| // Can't pre-allocate anything, because we don't know the string length yet... | ||
| std::vector<char16_t> utf16Buffer; |
There was a problem hiding this comment.
You can, however, reserve bufferSize/sizeof(char16_t) to flatten the number of actual memory allocations to 1.
There was a problem hiding this comment.
hmm, I don't think that's a good idea in the general case. For something like the file server manifests, where we have potentially hundreds of zero-terminated strings concatenated, we don't want to reserve the size of the entire manifest for each string.
There was a problem hiding this comment.
Ah, good point, I wasn't considering the case where the buffer is significantly larger than the string to extract from it...
There was a problem hiding this comment.
Would something like 128 work as an initial size that's big enough to handle most of the strings we'd expect over the network without incurring reallocations but also not incurring a big chunk of memory?
There was a problem hiding this comment.
I rewrote the code so it counts the string length beforehand. This allows reusing hsSTStringFromUTF16LE, which allocates a buffer with the right size.
Sources/Plasma/CoreLib/hsEndian.cpp
Outdated
| return ST::string::from_utf16(utf16Buffer.data(), utf16Buffer.size()); | ||
| } | ||
|
|
||
| ST::utf16_buffer hsSTStringToUTF16LE(const ST::string& string) |
There was a problem hiding this comment.
Swapping the endianness of the buffer implies that this is no longer a legal utf16_buffer... It may make more sense logically to return as a vector<uint16_t>
There was a problem hiding this comment.
I made it a std::vector<uint8_t> now, so we don't have any incorrect endianness values at all. This actually also works better for the calling code.
Sources/Plasma/CoreLib/hsEndian.h
Outdated
|
|
||
| #include "HeadSpin.h" | ||
|
|
||
| #include <string_theory/string> |
There was a problem hiding this comment.
Changing the return type of hsSTStringToUTF16LE as suggested above would also resolve this ;)
dgelessus
force-pushed
the
pnNpCommon_endianness
branch
from
cd6cbe1 to
2c39b17
Compare
This removes one dependency on pnUtils/StrCopy.
This makes the most sense for a caller that wants to skip past the string that was just read.
This way, it can also share the core code with hsSTStringFromUTF16LE.
dgelessus
force-pushed
the
pnNpCommon_endianness
branch
from
65326ec to
2bec282
Compare
4 participants
My goal was just to remove the
StrCopycalls, but while I was here, I also fixed the endianness and alignment issues in this code. Perhaps it will help @dpogue on PowerPC :)This adds some helper functions for converting little-endian UTF-16 directly from/to
ST::string, which could also be useful for other code that reads/writes UTF-16 strings manually, like file server manifests and the GameMgr.