Skip to content

Commit 6c93bf7

Browse files
committed
Silence uninitialized-value warnings in compareJsonbContainers().
Because not every path through JsonbIteratorNext() sets val->type, some compilers complain that compareJsonbContainers() is comparing possibly-uninitialized values. The paths that don't set it return WJB_DONE, WJB_END_ARRAY, or WJB_END_OBJECT, so it's clear by manual inspection that the "(ra == rb)" code path is safe, and indeed we aren't seeing warnings about that. But the (ra != rb) case is much less obviously safe. In Assert-enabled builds it seems that the asserts rejecting WJB_END_ARRAY and WJB_END_OBJECT persuade gcc 15.x not to warn, which makes little sense because it's impossible to believe that the compiler can prove of its own accord that ra/rb aren't WJB_DONE here. (In fact they never will be, so the code isn't wrong, but why is there no warning?) Without Asserts, the appearance of warnings is quite unsurprising. We discussed fixing this by converting those two Asserts into pg_assume, but that seems not very satisfactory when it's so unclear why the compiler is or isn't warning: the warning could easily reappear with some other compiler version. Let's fix it in a less magical, more future-proof way by changing JsonbIteratorNext() so that it always does set val->type. The cost of that should be pretty negligible, and it makes the function's API spec less squishy. Reported-by: Erik Rijkers <er@xs4all.nl> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl Discussion: https://postgr.es/m/0c623e8a204187b87b4736792398eaf1@postgrespro.ru Backpatch-through: 13
1 parent 0e2cc38 commit 6c93bf7

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

src/backend/utils/adt/jsonb_util.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,6 @@ compareJsonbContainers(JsonbContainer *a, JsonbContainer *b)
263263
else
264264
{
265265
/*
266-
* It's safe to assume that the types differed, and that the va
267-
* and vb values passed were set.
268-
*
269266
* If the two values were of the same container type, then there'd
270267
* have been a chance to observe the variation in the number of
271268
* elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
@@ -788,15 +785,20 @@ JsonbIteratorInit(JsonbContainer *container)
788785
* It is our job to expand the jbvBinary representation without bothering them
789786
* with it. However, clients should not take it upon themselves to touch array
790787
* or Object element/pair buffers, since their element/pair pointers are
791-
* garbage. Also, *val will not be set when returning WJB_END_ARRAY or
792-
* WJB_END_OBJECT, on the assumption that it's only useful to access values
793-
* when recursing in.
788+
* garbage.
789+
*
790+
* *val is not meaningful when the result is WJB_DONE, WJB_END_ARRAY or
791+
* WJB_END_OBJECT. However, we set val->type = jbvNull in those cases,
792+
* so that callers may assume that val->type is always well-defined.
794793
*/
795794
JsonbIteratorToken
796795
JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
797796
{
798797
if (*it == NULL)
798+
{
799+
val->type = jbvNull;
799800
return WJB_DONE;
801+
}
800802

801803
/*
802804
* When stepping into a nested container, we jump back here to start
@@ -834,6 +836,7 @@ JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
834836
* nesting).
835837
*/
836838
*it = freeAndGetParent(*it);
839+
val->type = jbvNull;
837840
return WJB_END_ARRAY;
838841
}
839842

@@ -887,6 +890,7 @@ JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
887890
* of nesting).
888891
*/
889892
*it = freeAndGetParent(*it);
893+
val->type = jbvNull;
890894
return WJB_END_OBJECT;
891895
}
892896
else
@@ -931,8 +935,10 @@ JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
931935
return WJB_VALUE;
932936
}
933937

934-
elog(ERROR, "invalid iterator state");
935-
return -1;
938+
elog(ERROR, "invalid jsonb iterator state");
939+
/* satisfy compilers that don't know that elog(ERROR) doesn't return */
940+
val->type = jbvNull;
941+
return WJB_DONE;
936942
}
937943

938944
/*

0 commit comments

Comments
 (0)