rurban / safeclib

safec libc extension with all C11 Annex K functions
https://rurban.github.io/safeclib/
Other
329 stars 64 forks source link

Non-portable format strings #138

Closed mschulz-at-hilscher closed 5 months ago

mschulz-at-hilscher commented 5 months ago

When compiling for systems where types such as int32_t are unequal to int (e.g., long int), then compilation fails. Please, use the defines from inttypes.h when defining format strings.

For example:

..\..\Components\safeclib\safeclib-core\tests\test_memcmp_s.c:193:35: error: format '%d' expects argument of type 'int', but argument 4 has type 'int32_t {aka long int}' [-Werror=format=]
         debug_printf("%s %u  Ind=%d  rc=%u \n", __FUNCTION__, __LINE__, ind,
                                  ~^
                                  %ld

If you want, I could also create a PR with the required changes. During configure one would need to check that inttypes.h is available and otherwise fallback to default definitions assuing that int32_t equals int, for example.

Alternatively, one could define variables as int instead of int32_t if this type is expected by the APIs anyways.

mschulz-at-hilscher commented 5 months ago

Btw. where is memcmp_s defined in the C standard? I did not find it in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1172.pdf

I was wondering why diff in memcmp_s(const void *dest, rsize_t dmax, const void *src, rsize_t slen, int *diff) is defined as int and not as ssize_t for example.

rurban commented 5 months ago

PR would be nice, thanks.

Ad ssize_t: Our memcmp_is an extension, thus only int. Preventing DOS. https://en.cppreference.com/w/c/string/byte/memcmp

mschulz-at-hilscher commented 5 months ago

Ad ssize_t: Our memcmp_is an extension, thus only int. Preventing DOS. https://en.cppreference.com/w/c/string/byte/memcmp

Shall I then change the tests to use ints instead of int32_ts in the tests when I create a PR? Or would it be better to get rid of int completely and change the API to use ssize_t instead?

rurban commented 5 months ago

Yes, int please