From b7831865159d5fb6f0d263e6023f0986589fe254 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Sat, 16 Mar 2024 23:18:28 +0100 Subject: [PATCH] Add destroyStringInfo function for cleaning up StringInfos destroyStringInfo() is a counterpart to makeStringInfo(), freeing a palloc'd StringInfo and its data. This is a convenience function to align the StringInfo API with the PQExpBuffer API. Originally added in the OAuth patchset, it was extracted and committed separately in order to aid upcoming JSON work. Author: Daniel Gustafsson Author: Jacob Champion Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/CAOYmi+mWdTd6ujtyF7MsvXvk7ToLRVG_tYAcaGbQLvf=N4KrQw@mail.gmail.com --- 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/stringinfo.c | 16 ++++++++++++++++ src/include/lib/stringinfo.h | 9 ++++++++- src/test/regress/pg_regress.c | 3 +-- 8 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index 5fbbe5ffd20..5fea86ad0fd 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 a05d69922d9..5a47fa984df 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 a5e48744acb..a941654d5a3 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 beecd0c2ac1..3e4ca874d81 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 6f0814d9ac6..74f8be9eeac 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/common/stringinfo.c b/src/common/stringinfo.c index c61d5c58f30..ec5fc2422d8 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 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/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 2cd636b01cf..cd9632e3fca 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/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f1f6011ae0a..76f01c73f56 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); } /*