Re: strange cppcheck finding

[Willy Tarreau]

On Tue, Mar 20, 2018 at 06:45:08PM +0500,  ??? wrote:
> "UB" stands for undefined behaviour. that's the reason why cppcheck is
> unhappy.
> how do that properly - that's the question :)

The thing is that I'm not aware of any other way to safely detect integer
overflows, it's always done like this. In fact this undefined behaviour
on unsigned ints is defined per-architecture. I think you can safely turn
this one off as we do use integer wrapping at other places on purpose, and
we even build with -fwrapv to make it defined :-)

Cheers,
Willy

Re: strange cppcheck finding

[Илья Шипицин]

"UB" stands for undefined behaviour. that's the reason why cppcheck is
unhappy.
how do that properly - that's the question :)

2018-03-20 10:48 GMT+05:00 Willy Tarreau :

> On Mon, Mar 19, 2018 at 06:55:46PM +0500,  ??? wrote:
> > (it's master)
> >
> > is it in purpose ?
> >
> > [src/ssl_sock.c:1553]: (warning) Invalid test for overflow
> > 'msg+rec_len and
> > overflow is UB.
>
> The code is :
>
> rec_len = (msg[0] << 8) + msg[1];
> msg += 2;
> if (msg + rec_len > end || msg + rec_len < msg)
> return;
>
> It's indeed an overflow check which was placed on purpose. What does
> your tool propose as a better way to check for an overflow ? rec_len
> being a size_t, it's unsigned so the overflow check is fine and
> necessary in my opinion.
>
> Regards,
> Willy
>

Re: strange cppcheck finding

[Willy Tarreau]

On Mon, Mar 19, 2018 at 06:55:46PM +0500,  ??? wrote:
> (it's master)
> 
> is it in purpose ?
> 
> [src/ssl_sock.c:1553]: (warning) Invalid test for overflow
> 'msg+rec_len overflow is UB.

The code is :

rec_len = (msg[0] << 8) + msg[1];
msg += 2;
if (msg + rec_len > end || msg + rec_len < msg)
return;

It's indeed an overflow check which was placed on purpose. What does
your tool propose as a better way to check for an overflow ? rec_len
being a size_t, it's unsigned so the overflow check is fine and
necessary in my opinion.

Regards,
Willy

strange cppcheck finding

[Илья Шипицин]

(it's master)

is it in purpose ?

[src/ssl_sock.c:1553]: (warning) Invalid test for overflow
'msg+rec_len