Wireshark mailing list archives

size_t vs int


From: Dario Lombardo <dario.lombardo.ml () gmail com>
Date: Fri, 4 Sep 2015 15:46:57 +0200

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.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

Current thread: