Re: jsonb iterator not fully initialized

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: jsonb iterator not fully initialized
Date: 2018-05-27 20:40:53
Message-ID: CA+q6zcWS6GJgFL+SjJG0U47DnUrupM3b9wS61ABDFrrh+hwf-A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 26 May 2018 at 16:47, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
> On 05/26/2018 03:09 AM, Piotr Stefaniak wrote:
>> On 2018-05-26 02:02, Peter Eisentraut wrote:
>>>
>>> It can be fixed this way:
>>>
>>> --- a/src/backend/utils/adt/jsonb_util.c
>>> +++ b/src/backend/utils/adt/jsonb_util.c
>>> @@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
>>> JsonbIterator *parent)
>>> {
>>> JsonbIterator *it;
>>>
>>> - it = palloc(sizeof(JsonbIterator));
>>> + it = palloc0(sizeof(JsonbIterator));
>>> it->container = container;
>>> it->parent = parent;
>>> it->nElems = JsonContainerSize(container);
>>>
>>> It's probably not a problem in practice, since the isScalar business is
>>> apparently only used in the array case, but it's dubious to leave things
>>> uninitialized like this nonetheless.

Yes, sounds reasonable.

>> I've seen it earlier but couldn't decide what my proposed fix should
>> look like. One of the options I considered was:
>>
>> --- a/src/backend/utils/adt/jsonfuncs.c
>> +++ b/src/backend/utils/adt/jsonfuncs.c
>> @@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void
>> *action_state,
>> JsonbIteratorToken type;
>> JsonbParseState *st = NULL;
>> text *out;
>> - bool is_scalar = false;
>>
>> it = JsonbIteratorInit(&jsonb->root);
>> - is_scalar = it->isScalar;
>>
>> while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
>> {
>> @@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
>> *action_state,
>> }
>>
>> if (res->type == jbvArray)
>> - res->val.array.rawScalar = is_scalar;
>> + res->val.array.rawScalar =
>> JsonContainerIsScalar(&jsonb->root);
>>
>> return JsonbValueToJsonb(res);
>> }
>
> palloc0 seems cleaner.

Totally agree, palloc0 looks better (although I assume it's going to be
negligibly slower in those cases that aren't affected by this problem).

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2018-05-27 21:09:58 Re: SP-GiST failing to complete SP-GiST index build
Previous Message Andres Freund 2018-05-27 20:00:06 Re: found xmin from before relfrozenxid on pg_catalog.pg_authid