On 05/10/2014 01:32 AM, Tom Lane wrote:
> Peter Geoghegan <pg@heroku.com> writes:
>> On Fri, May 9, 2014 at 2:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> However, what it looks to me like we've got here is a very bad
>>> reimplementation of StringInfo buffers. There is for example no
>>> integer-overflow checking here. Rather than try to bring this code
>>> up to speed, I think we should rip it out and use StringInfo.
>
>> Heikki did specifically consider StringInfo buffers and said they were
>> not best suited to the task at hand. At the time I thought he meant
>> that he'd do something domain-specific to avoid unnecessary geometric
>> growth in the size of the buffer (I like to grow buffers to either
>> twice their previous size, or just big enough to fit the next thing,
>> whichever is larger), but that doesn't appear to be the case. Still,
>> it would be good to know what he meant before proceeding. It probably
>> had something to do with alignment.
>
> It looks to me like he wanted an API that would let him reserve space
> separately from filling it, which is not in stringinfo.c but is surely
> easily built on top of it.
Right, the API to reserve space separately was what I had in mind.
> For the moment, I've just gotten rid of
> the buggy code fragment in favor of calling enlargeStringInfo, which
> I trust to be right.
Thanks. I admit it didn't even occur to me to keep the localized API in
jsonb_utils as wrappers around appendString* functions. I only
considered two options: using appendString* directly, or doing
repalloc's in jsonb_utils.c. I like what you did there.
- Heikki