From 78bfbfe3cbdfd4095bc9ef220dfa11dfdc404556 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 14 Mar 2024 16:58:06 +0900 Subject: [PATCH v3 1/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 Jacob Champion. Co-authored-by: Daniel Gustafsson --- src/include/lib/stringinfo.h | 9 ++++++++- 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/common/stringinfo.c | 18 +++++++++++++++++- src/bin/pg_combinebackup/pg_combinebackup.c | 5 +---- src/test/regress/pg_regress.c | 3 +-- 8 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 2cd636b01c..cd9632e3fc 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 a StringInfo and its buffer (opposite of makeStringInfo()). + */ +extern void destroyStringInfo(StringInfo str); + #endif /* STRINGINFO_H */ 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/common/stringinfo.c b/src/common/stringinfo.c index c61d5c58f3..2910799bfd 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -70,7 +70,7 @@ initStringInfo(StringInfo str) * * Reset the StringInfo: the data buffer remains valid, but its * previous content, if any, is cleared. - * +* * Read-only StringInfos as initialized by initReadOnlyStringInfo cannot be * reset. */ @@ -350,3 +350,19 @@ enlargeStringInfo(StringInfo str, int needed) str->maxlen = newlen; } + +/* + * destroyStringInfo + * + * Frees a StringInfo and its buffer (opposite of makeStringInfo()). + * This must only be called on palloc'd StringInfos. + */ +void +destroyStringInfo(StringInfo str) +{ + /* don't allow destroys of read-only StringInfos */ + Assert(str->maxlen != 0); + + pfree(str->data); + pfree(str); +} diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 6f0814d9ac..74f8be9eea 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -526,10 +526,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/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.43.0