Thanks for taking a look at this. On Wed, 4 Oct 2023 at 16:57, Michael Paquier <mich...@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
