Thanks for taking a look at this.
On Wed, 4 Oct 2023 at 16:57, Michael Paquier <michael@paquier.xyz> wrote:
> + buf.len = VARSIZE_ANY_EXHDR(sstate);
> + buf.maxlen = 0;
> + buf.cursor = 0;
>
> Perhaps it would be worth hiding that in a macro defined in
> stringinfo.h?
The original patch had a new function in stringinfo.c which allowed a
StringInfoData to be initialised from an existing string with some
given length. Tom wasn't a fan of that because there wasn't any
protection against someone trying to use the given StringInfoData and
then calling appendStringInfo to append another string. That can't be
done in this case as we can't repalloc the VARDATA_ANY(state) pointer
due to it not pointing directly to a palloc'd chunk. Tom's complaint
seemed to be about having a reusable function which could be abused,
so I modified the patch to remove the reusable code. I think your
macro idea in stringinfo.h would put the patch in the same position as
it was initially.
It would be possible to do something like have maxlen == -1 mean that
the StringInfoData.data field isn't being managed internally in
stringinfo.c and then have all the appendStringInfo functions check
for that, but I really don't want to add overhead to everything that
uses appendStringInfo just for this.
David