From 0e3db5e04b094fb1bb6d5e497e565659acdde87a Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Wed, 13 Mar 2024 09:35:19 -0700 Subject: [PATCH v2 2/2] Add a helper function for cleaning up StringInfos destroyStringInfo() is a counterpart to makeStringInfo(): it frees a palloc'd StringInfo and its data. API originally by Daniel Gustafsson, with docs and tweaks added by me. Co-authored-by: Daniel Gustafsson --- src/backend/backup/basebackup.c | 3 +-- src/backend/commands/subscriptioncmds.c | 3 +-- src/backend/utils/adt/jsonb.c | 3 +-- src/backend/utils/adt/xml.c | 6 ++---- src/bin/pg_combinebackup/pg_combinebackup.c | 5 +---- src/common/jsonapi.c | 10 ++-------- src/common/stringinfo.c | 16 ++++++++++++++++ src/include/lib/stringinfo.h | 9 ++++++++- src/test/regress/pg_regress.c | 3 +-- 9 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 5fbbe5ffd2..5fea86ad0f 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -397,8 +397,7 @@ perform_base_backup(basebackup_options *opt, bbsink *sink, endtli = backup_state->stoptli; /* Deallocate backup-related variables. */ - pfree(tablespace_map->data); - pfree(tablespace_map); + destroyStringInfo(tablespace_map); pfree(backup_state); } PG_END_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, BoolGetDatum(false)); diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index a05d69922d..5a47fa984d 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -506,8 +506,7 @@ check_publications(WalReceiverConn *wrconn, List *publications) appendStringInfoChar(cmd, ')'); res = walrcv_exec(wrconn, cmd->data, 1, tableRow); - pfree(cmd->data); - pfree(cmd); + destroyStringInfo(cmd); if (res->status != WALRCV_OK_TUPLES) ereport(ERROR, diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index a5e48744ac..a941654d5a 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -133,8 +133,7 @@ jsonb_send(PG_FUNCTION_ARGS) pq_begintypsend(&buf); pq_sendint8(&buf, version); pq_sendtext(&buf, jtext->data, jtext->len); - pfree(jtext->data); - pfree(jtext); + destroyStringInfo(jtext); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index beecd0c2ac..3e4ca874d8 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -2163,8 +2163,7 @@ xml_errorHandler(void *data, PgXmlErrorPtr error) appendBinaryStringInfo(&xmlerrcxt->err_buf, errorBuf->data, errorBuf->len); - pfree(errorBuf->data); - pfree(errorBuf); + destroyStringInfo(errorBuf); return; } @@ -2195,8 +2194,7 @@ xml_errorHandler(void *data, PgXmlErrorPtr error) (errmsg_internal("%s", errorBuf->data))); } - pfree(errorBuf->data); - pfree(errorBuf); + destroyStringInfo(errorBuf); } diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 31ead7f405..1f8a90fb9e 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -504,10 +504,7 @@ check_backup_label_files(int n_backups, char **backup_dirs) /* Free memory that we don't need any more. */ if (lastbuf != buf) - { - pfree(buf->data); - pfree(buf); - } + destroyStringInfo(buf); /* * Return the data from the first backup_info that we read (which is the diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c index 6243426f5f..ec58109837 100644 --- a/src/common/jsonapi.c +++ b/src/common/jsonapi.c @@ -186,16 +186,10 @@ void freeJsonLexContext(JsonLexContext *lex) { if (lex->flags & JSONLEX_FREE_STRVAL) - { - pfree(lex->strval->data); - pfree(lex->strval); - } + destroyStringInfo(lex->strval); if (lex->errormsg) - { - pfree(lex->errormsg->data); - pfree(lex->errormsg); - } + destroyStringInfo(lex->errormsg); if (lex->flags & JSONLEX_FREE_STRUCT) pfree(lex); diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index c61d5c58f3..da1f3fb5ad 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -350,3 +350,19 @@ enlargeStringInfo(StringInfo str, int needed) str->maxlen = newlen; } + +/* + * destroyStringInfo + * + * Frees the StringInfo and its buffer (the opposite of makeStringInfo()). This + * must only be called on palloc'd StringInfos. + */ +void +destroyStringInfo(StringInfo str) +{ + /* read-only StringInfos must not be destroyed */ + Assert(str->maxlen != 0); + + pfree(str->data); + pfree(str); +} diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 2cd636b01c..156a83692d 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -87,7 +87,8 @@ typedef StringInfoData *StringInfo; * to be len + 1 in size. * * To destroy a StringInfo, pfree() the data buffer, and then pfree() the - * StringInfoData if it was palloc'd. There's no special support for this. + * StringInfoData if it was palloc'd. (For StringInfos created with + * makeStringInfo(), destroyStringInfo() is provided for this purpose.) * However, if the StringInfo was initialized using initReadOnlyStringInfo() * then the caller will need to consider if it is safe to pfree the data * buffer. @@ -233,4 +234,10 @@ extern void appendBinaryStringInfoNT(StringInfo str, */ extern void enlargeStringInfo(StringInfo str, int needed); +/*------------------------ + * destroyStringInfo + * Frees the StringInfo and its buffer (the opposite of makeStringInfo()). + */ +extern void destroyStringInfo(StringInfo str); + #endif /* STRINGINFO_H */ diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f1f6011ae0..76f01c73f5 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -1174,8 +1174,7 @@ psql_end_command(StringInfo buf, const char *database) } /* Clean up */ - pfree(buf->data); - pfree(buf); + destroyStringInfo(buf); } /* -- 2.34.1