Fix MD5 digest

[Semphriss]

Commit c04b46a mistakenly used snprintf(str, sizeof(str), ...) with sizeof instead of strlen.

Since using strlen would add a header, and since we know both the buffer length and the length of the printed string, the size can simply be hardcoded (2 characters + 1 terminating NULL character, which will be overwritten for each printf except the last).

I've also removed the now-unneeded manual terminating null byte.

[swagtoy]

First off, this is the wrong commit, Tobbi simply fixed linting in this one.

Second of all, If you're going to out of your way to refactor some weird logic in this library, you might as well just use std::stringstream for this instead of snprintf. Or hell, even just std::fmt might be the preferred solution, but alas:

std::stringstream res;
res << std::hex;
for ... {
    res << digest[i];
}

It is slightly slower due to no preallocation but more c++'y and less prone to any issues (i mean... we're already casting it to a string at the end anyway!!!). No need to use these C functions...

And in all honesty we..... shouldn't be relying on a hex digest to begin with :-) We should just rely on the raw md5sum data through the rest of the code. In the usage at Addon::get_md5(), it would be preferred if get_md5() returned the array of each byte, then we either used std::equal or just some std::array comparison to do this. We shall never need to rely directly on the stringified checksum data. It is designed for reading and simpler string comparisons, not for actual code comparisons.

Third of all, you can move so to the line itself now.

I personally believe we should just use string stream since we are already doing a std::string comparison at the end anyway. There is no need to call snprintf by hand like this for us. I very personally we shouldn't even use this function at all...

[tulpenkiste]

Second of all, If you're going to out of your way to refactor some weird logic in this library, you might as well just use std::stringstream for this instead of snprintf. Or hell, even just std::fmt might be the preferred solution, but alas

As far as I'm aware, std::fmt is not a viable solution for SuperTux due to needing to support older Ubuntu versions which don't have C++20

[swagtoy]

We have our own format library in the game. Not a c++20 thing

…

On Sat, Dec 14, 2024, 3:56 PM Tulpen ***@***.***> wrote: Second of all, If you're going to out of your way to refactor some weird logic in this library, you might as well just use std::stringstream for this instead of snprintf. Or hell, even just std::fmt might be the preferred solution, but alas As far as I'm aware, std::fmt is not a viable solution for SuperTux due to needing to support older Ubuntu versions which don't have C++20 — Reply to this email directly, view it on GitHub <#3136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AY22OG7YDCHFNUQJK2ED75D2FSLOTAVCNFSM6AAAAABTHTUFQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBTGM2DEMRTGM> . You are receiving this because you commented.Message ID: ***@***.***>