Translated buffers are early null terminated

[Adrianilloo]

I've just noticed what seems to be an important issue.

When holding a string buffer of size N, formatting it without involving translations allow for the null terminator to be placed at the last cell as much (N - 1), correctly. However, when a translation is used while formatting, the null terminator is written as much late at the N - 2 cell. This causes to always render one cell of the max. length useless.

See attached picture for an illustrative sample.

Thank you.

[Wend4r]

https://github.com/alliedmodders/sourcemod/blob/f603b7aec3219e0db56882447b35a57afe4c6d8a/core/logic/smn_string.cpp#L211-L222 https://github.com/alliedmodders/sourcemod/blob/1fbe5e1daaee9ba44164078fe7f59d862786e612/core/logic/sprintf.cpp#L1033 https://github.com/alliedmodders/sourcemod/blob/1fbe5e1daaee9ba44164078fe7f59d862786e612/core/logic/sprintf.cpp#L1221 https://github.com/alliedmodders/sourcemod/blob/1fbe5e1daaee9ba44164078fe7f59d862786e612/core/logic/sprintf.cpp#L145

It seems to me that this is the problem with a recursive atcprintf call by formatting translations. When inserting a translation, the last non-terminated character is cut off (size_t llen = maxlen - 1)

[Wend4r]

@dvander, if you have time to fix it, then I also had a problem with the fact that when throwing an exception from atcprintf it did not terminate running the SP code, I think this is not in the SM style. I was too lazy to write an issue about this, sorry.

[dvander]

At this point, it's probably a compatibility issue to fix that bug :( - the exception one, that is.

[dvander]

@Wend4r if you have an example of how this is broken, let me know, I can investigate.

[Wend4r]

@Wend4r if you have an example of how this is broken, let me know, I can investigate.

decl char can be associated with a variable in which an unnecessary string.

[dvander]

Okay - in the future, actual code would be a lot better than a picture of code. This is documented as part of the API though, and you're not following the contract correctly. FormatNativeString returns an error code and it's your responsibility to propagate it via ThrowNativeError if desired. There was a bug introduced where the error code isn't correct though, and I'll fix that now.

[Adrianilloo]

Sorry to ask, but are there any news regarding the main issue?

[dvander]

My guess is that this line in sprintf.cpp:

res = Translate(buf_p, llen, pCtx, key, *target, params, &arg, &error);

Should be:

res = Translate(buf_p, llen + 1, pCtx, key, *target, params, &arg, &error);

atcprintf wants a buffer including the null terminator, but llen counts remaining non-null characters. Feel free to give that a try (I don't have the time at the moment to submit a PR and test).

[1] https://cs.alliedmods.net/sourcemod/source/core/logic/sprintf.cpp#1221

[Adrianilloo]

Ok,

I compiled SM on the master branch (aka 1.11 currently), deployed the binaries to my test server and the initial showcasing SP test now fully includes all characters in the translated buffer:

So I can say: it works!

Not sure if the change would have any other implication. Let me know if I shall create a PR (to which branch?) or you'll handle it. Have in mind I'd be interested in the patch to be relased into SM 1.10, at least (not sure if that's assumed).

[dvander]

Good to hear, a PR would be great.