Operands don't affect result (CONSTANT_EXPRESSION_RESULT) (src/backend/utils/adt/jsonfuncs.c)
Hi,
Per Coverity.
The use of type "long" is problematic with Windows 64bits.
Long type on Windows 64bits is 32 bits.
See at:
https://docs.microsoft.com/pt-br/cpp/cpp/data-type-ranges?view=msvc-160
*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)
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
Attachments:
jsonfuncs.patchapplication/octet-stream; name=jsonfuncs.patchDownload
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;
/*
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
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
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
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