[3rdParty] Pine: Replace sprintf with snprintf to silence compiler warning

[shinra-electric]

This fixes the following compiler deprecation warning

warning: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead.

[shinra-electric]

This should probably be checked by @GovanifY

[GovanifY]

You use sizeof on the yet-uninitialized variable. This leads it to return an undefined value (which will probably be 0, as uninitialized values usually are set to 0 but the array could as well be, say, filled with 0xCC for debugging making the size unpredictable and past the array's size).

The way you propose it is unsafe but doing a sizeof(Impl::get_version_and_branch().c_str()) would be a good idea! You might also want to store the c_str only once to avoid having to do the same calls a few times. IIRC c_str() does not make a copy so at least you won't waste memory by doing this, just cycles, which might be able to be opitmized out by the compiler so up to you on that one.

[elad335]

Everything is safe with unitialized values unless you actually rely on their values to be specific, and sizeof doesn't care about it.

Example:

int r;
int d = r - r:

d is 0 and there is no undefined behavior here.

[GovanifY]

The sizeof is in this case either used directly on a pointer (

					char* hash = new char[size];
					snprintf(hash, sizeof(hash), "%s", hash_string.c_str());

) which returns the size of the pointer or the empty array.
In the case of version (empty array) I forgot it was compile-time in this case, as version is stored in the stack. Still breaks past behaviour on pointers though.

[elad335]

Yeah he needs to write simply size there.

[GovanifY]

See for yourself what I mean:

➜  tmp (nix-shell) cat test.cpp                                                                                                                                                                                  gcc
#include <stdio.h>
#include <string>

int main() {

    char* hash = new char[256];
    printf("%lu", sizeof(hash));
    return 0;
}
➜  tmp (nix-shell) g++ test.cpp                                                                                                                                                                                  gcc
➜  tmp (nix-shell) ./a.out                                                                                                                                                                                       gcc
8⏎                       

For sizeof, I misconstrued it as C's behaviour, which is NOT always compile-time (and thus safe in this case), so that one's on me (or even more probably as an strlen).

[GovanifY]

Yeah he needs to write simply size there.

Yeah, he/you (depending on who is reading) can just use the size directly from the std::string.

[shinra-electric]

Sorry but I'm not a C++ dev and am not very knowledgable about it.

How exactly do you recommend for me to amend it? Writing size where?

[GovanifY]

The = 0x00; are useless as snprintf nul-terminates, and had an off-by-one error (which should have been caught by ASAN when developing the server, but I did not commit this port).