On 15.04.21 15:55, Willy Tarreau wrote:
On Thu, Apr 15, 2021 at 03:41:18PM +0200, Aleksandar Lazic wrote:
Now when I remove the check "smp->data.u.sint < 0" every positive value is
bigger then JSON INT_MIN and returns 0.
But don't you agree that this test DOES nothing ? If it changes anything
it means the issue is somewhere else and is a hidden bug waiting to strike
and that we must address it.
Look:
if (smp->data.u.sint < 0 && smp->data.u.sint < JSON_INT_MIN)
is exactly equivalent to:
if (smp->data.u.sint < JSON_INT_MIN && smp->data.u.sint < 0)
JSON_INT_MIN < 0 so the first part implies the second one. Said differently,
there is no value of sint that validates <JSON_INT_MIN without validating <0.
I think it checks if the value is negative or positive and then verify if the
value is bigger then the the max allowed value, +/-.
Maybe I thing wrong, so let us work with concrete values.
```
printf("\n\n>> smp->data.u.sint :%lld: < JSON_INT_MIN :%lld: if-no-check:%d:<<\n",
smp->data.u.sint,JSON_INT_MIN,(smp->data.u.sint < JSON_INT_MIN));
printf( ">> smp->data.u.sint :%lld: > JSON_INT_MAX :%lld: if-no-check:%d:<<\n\n",
smp->data.u.sint,JSON_INT_MAX,(smp->data.u.sint > JSON_INT_MAX));
if (smp->data.u.sint < 0 && smp->data.u.sint < JSON_INT_MIN)
return 0;
else if (smp->data.u.sint > 0 && smp->data.u.sint > JSON_INT_MAX)
return 0;
```
Input is here 4.
smp->data.u.sint :4: < JSON_INT_MIN :-9007199254740991: if-no-check:1:<<
smp->data.u.sint :4: > JSON_INT_MAX :9007199254740991: if-no-check:0:<<
Input is here -4.
smp->data.u.sint :-4: < JSON_INT_MIN :-9007199254740991: if-no-check:0:<<
smp->data.u.sint :-4: > JSON_INT_MAX :9007199254740991: if-no-check:1:<<
OK I think I got it. It's just because your definitions of JSON_INT_MIN
and JSON_INT_MAX are unsigned and the comparison is made in unsigned mode.
So when you do "4 < JSON_INT_MIN" it's in fact "4 < 2^64-(1<53)-1" so it's
true. And conversely for the other one.
I'm pretty sure that if you change your constants to:
#define JSON_INT_MAX ((1LL << 53) - 1)
#define JSON_INT_MIN (-JSON_INT_MAX)
It will work :-)
Well I don't think so because 4 is still bigger then -9007199254740991 ;-)
Never the less I have changed the defines and rerun the tests.
Btw, this vtest is a great enhancement to haproxy ;-)
```
#define JSON_INT_MAX ((1ULL << 53) - 1)
#define JSON_INT_MIN (-JSON_INT_MAX)
printf("\n\n>> smp->data.u.sint :%lld: < JSON_INT_MIN :%lld: if-no-check:%d:<<\n",
smp->data.u.sint,JSON_INT_MIN,(smp->data.u.sint < JSON_INT_MIN));
printf( ">> smp->data.u.sint :%lld: > JSON_INT_MAX :%lld: if-no-check:%d:<<\n\n",
smp->data.u.sint,JSON_INT_MAX, (smp->data.u.sint > JSON_INT_MAX));
if (smp->data.u.sint < JSON_INT_MIN)
return 0;
else if (smp->data.u.sint > JSON_INT_MAX)
return 0;
```
>> smp->data.u.sint :4: < JSON_INT_MIN :-9007199254740991: if-no-check:1:<<
>> smp->data.u.sint :4: > JSON_INT_MAX :9007199254740991: if-no-check:0:<<
That's among the historical idiocies of the C language that considers
the signedness as part of the variable instead of being the mode of the
operation applied to the variable. This results in absurd combinations.
Willy
