libsignal-protocol-c icon indicating copy to clipboard operation
libsignal-protocol-c copied to clipboard

Fix some printf formatting codes for size_t/ssize_t.

Open Smattr opened this issue 8 years ago • 2 comments

This commit switches to using "%zu" and "%zd" for printing size_t and ssize_t, respectively. These format codes are more portable than "%u" or "%d". For example, on x86_64 size_ts and ssize_ts are 64-bit values, while ints are 32-bit values. In practice, this does not cause problems in the x86_64 System V ABI because 32-bit values are naturally extended in this scenario, but on other platforms it can cause stack corruption.


~~If you're happy to accept this, I believe I need to sign a CLA, but it's not clear to me where to get this from. Are you able to point me in the right direction? Thanks.~~

Also, I inadvertently picked this up after slapping __attribute__((format(printf, 3, 4))) on the signal_log declaration. I assumed you don't want GNU-isms like this in the code base so I didn't commit this as well, but please let me know if you'd like me to include that change as well.

Smattr avatar Jun 28 '16 00:06 Smattr

Unfortunately, the "z" length specifier appears to have been added in C99. As this library needs to maintain compatibility with certain C89 compilers, implementing a change like this one would also require adding some additional checks to make sure its supported by the compiler. Given that the library does not currently use a generated "config.h" file, such a check would probably have to be implemented by making cmake define something if the compiler supports "z".

dkonigsberg avatar Jun 29 '16 15:06 dkonigsberg

Interesting, I was not aware of this nuance. If maintaining C89 compatibility is a concern, my inclination would be not to do these changes but to instead cast the affected parameters to int when calling printf. What do you think?

Smattr avatar Jul 01 '16 05:07 Smattr