Thread: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
From
Ranier Vilela
Date:
Hi,
Per Coverity.
The use of type "long" is problematic with Windows 64bits.
Long type on Windows 64bits is 32 bits.
See at:
Therefore long never be > INT_MAX at Windows 64 bits. Thus lindex is always false in this expression: if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX || lindex < INT_MIN) Patch suggestion to fix this. diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 215a10f16e..54b0eded76 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -1675,7 +1675,7 @@ push_path(JsonbParseState **st, int level, Datum *path_elems, * end, the access index must be normalized by level. */ enum jbvType *tpath = palloc0((path_len - level) * sizeof(enum jbvType)); - long lindex; + int64 lindex; JsonbValue newkey; /* regards, Ranier Vilela |
Attachment
Re: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
From
Tom Lane
Date:
Ranier Vilela <ranier.vf@gmail.com> writes:
> *long* 4 *long int*, *signed long int* -2.147.483.648 a 2.147.483.647
> Therefore long never be > INT_MAX at Windows 64 bits.
> Thus lindex is always false in this expression:
> if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX || lindex
> < INT_MIN)
Warnings about this are purest nanny-ism.
At the same time, I think this code could be improved; but the way
to do that is to use strtoint(), rather than kluging the choice of
datatype even further.
regards, tom lane
Re: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
From
Ranier Vilela
Date:
Em qui., 11 de fev. de 2021 às 01:46, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> *long* 4 *long int*, *signed long int* -2.147.483.648 a 2.147.483.647
> Therefore long never be > INT_MAX at Windows 64 bits.
> Thus lindex is always false in this expression:
> if (errno != 0 || badp == c || *badp != '\0' || lindex > INT_MAX || lindex
> < INT_MIN)
At the same time, I think this code could be improved; but the way
to do that is to use strtoint(), rather than kluging the choice of
datatype even further.
No matter the function used strtol or strtoint, push_path will remain broken with Windows 64bits.
Or need to correct the expression.Definitely using long is a bad idea.
regards,
Ranier Vilela
Re: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
From
Tom Lane
Date:
Ranier Vilela <ranier.vf@gmail.com> writes:
> Em qui., 11 de fev. de 2021 às 01:46, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
>> At the same time, I think this code could be improved; but the way
>> to do that is to use strtoint(), rather than kluging the choice of
>> datatype even further.
> No matter the function used strtol or strtoint, push_path will remain
> broken with Windows 64bits.
There is quite a lot of difference between "broken" and "my compiler
generates pointless warnings". Still, I changed it to use strtoint(),
because that's simpler and better style.
regards, tom lane
Re: Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
From
Ranier Vilela
Date:
Em qui., 11 de fev. de 2021 às 14:51, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> Em qui., 11 de fev. de 2021 às 01:46, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
>> At the same time, I think this code could be improved; but the way
>> to do that is to use strtoint(), rather than kluging the choice of
>> datatype even further.
> No matter the function used strtol or strtoint, push_path will remain
> broken with Windows 64bits.
There is quite a lot of difference between "broken" and "my compiler
generates pointless warnings". Still, I changed it to use strtoint(),
because that's simpler and better style.
Thanks Tom, for fixing this.
regards,
Ranier Vilela