Hi list
I'm playing with afl and clang and I've found some points in the code where afl/clang complains, and I'd like to discuss how to change them with you.
A warning message got is
../codecs/sbc/sbc.c:111:16: warning: implicit conversion loses integer precision: 'size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
return size_out;
~~~~~~ ^~~~~~~~
../codecs/sbc/sbc.c:118:20: warning: implicit conversion loses integer precision: 'ssize_t' (aka 'long') to 'int' [-Wshorten-64-to-32]
framelen = sbc_decode(sbc, data_in, size_in, data_out, size_out, &len);
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(just an example). Clang is not wrong since
int
codec_sbc_decode(void *ctx, const void *input, int inputSizeBytes, void *output,
int *outputSizeBytes)
{
[...]
if (!output || !outputSizeBytes) {
return size_out; /* <<<======== we are returning a size_t, but return type is int */
}
[...]
}
is potentially dangerous. This is just a piece of code I took as example but there are loads of findings like this.
Now the question arises: which is the best way to patch the code?
The first possibility is to change the "lowest" function, to keep the current prototype but with some integer checks and casts.
The second possibility is to "sanitize" the lowest function (in the example above, AFAIK, there is no reason to have int as input/output, better have size_t), but this, like a domino effect, requires to change the calling function (in the example, change the codec_decode_fn typedef) that propagates the change to other functions and so on.
The first has less impact, but is more dirty. The second has great impact, but is more correct/elegant.
What do you think?
Dario.