I'm surprised I wasn't pinged on Discord or so since I seem to be the resident linux user of this mod. ^.^
But still, let my copy/paste a summary of what I put on Discord.
In short, the code posted above is:
Code:
if (tmp)
{
mbstate_t mbs;
mbrlen(NULL, 0, &mbs);
wcsrtombs(animPathName, &tmp, 1023, &mbs);
FAssertMsg(tmp == 0, "Failed to convert animation path name");
}
A couple of problems with this code. First let's look at the `wcsrtombs` function, it's return value isn't being checked. First of all it returns the length processed or it returns -1 on failure, both of those need to be checked for reasons I will get to shortly.
Second, the `tmp` is only altered by walking the pointer forward, so in the `FAssertMsg` the `tmp == 0` should only be false if it didn't successfully walk it the entire length, which can happen for two main reasons:
1. It can happen with the output destination, `animPathName` in this case, is too short to hold the entire path.
2. Invalid characters in the wide encoding.
I honestly doubt that #2 happened because wine/proton is very good about 'fixing up' the path, so that leaves #1.
So now why would #1 happen, well that would be because the full path to read that exceeds the 1023 buffer of `animPathName`, which is only 1024 length (1023 chars + null terminator).
So that then begs the question of how this would be read correctly, well first of all the `wcsrtombs` function is being misused here. First it should be used with a null initial argument and 0 length, this will return the size of the buffer needed (without the terminating null, so add 1 to it) or it will return `-1` if their are invalid multibyte characters. Then another `wcsrtombs` should be called with an appropriately sized buffer.
Because `wcsrtombs` is being misused according to the spec itself by using a hardcoded buffer length when the size of the converted string is unknown then it is entirely expected for this to fail, and you would also hit this assert on windows as well if Civ4 were installed into a long enough path.
So at the very least this code is wrong. `wcsrtombs` needs to be called once with a null destination and 0 length valure, there should be `FAssertMsg` on the return value that it is not `-1` (assuming you don't have an appropriate fallback, make sure to handle the failure case), then allocate a long enough buffer (a reusable `std::vector<char>` would probably be best so you can keep reusing that cache but it can grow as needed) to hold the data and call `wcsrtombs` again with the correct buffer length (+1 for null terminator).
The person that hits this assert should be able to work around it by making their path shorter and maybe making sure only ascii in the path just as a double-check.
I have Civ4 installed on a rather short path so that would explain why I haven't hit this issue.
