From 41b45576d532ab6ef4a968f78b5b06d700ebf61f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Mar 2020 21:36:53 -0400 Subject: [PATCH] Remove useless pfree()s at the ends of various ValuePerCall SRFs. We don't need to manually clean up allocations in a SRF's multi_call_memory_ctx, because the SRF_RETURN_DONE infrastructure takes care of that (and also ensures that it will happen even if the function never gets a final call, which simple manual cleanup cannot do). Hence, the code removed by this patch is a waste of code and cycles. Worse, it gives the impression that cleaning up manually is a thing, which can lead to more serious errors such as those fixed in commits 085b6b667 and b4570d33a. So we should get rid of it. These are not quite actual bugs though, so I couldn't muster the enthusiasm to back-patch. Fix in HEAD only. Justin Pryzby Discussion: https://postgr.es/m/20200308173103.GC1357@telsasoft.com --- contrib/pageinspect/btreefuncs.c | 15 ++++----------- contrib/pageinspect/ginfuncs.c | 4 ++-- contrib/pageinspect/hashfuncs.c | 7 ++----- src/backend/access/transam/multixact.c | 4 ---- src/backend/tsearch/wparser.c | 9 --------- src/backend/utils/adt/jsonfuncs.c | 14 -------------- src/backend/utils/adt/tsvector_op.c | 1 - 7 files changed, 8 insertions(+), 46 deletions(-) diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c index e6a2fc1e155..e7a323044bf 100644 --- a/contrib/pageinspect/btreefuncs.c +++ b/contrib/pageinspect/btreefuncs.c @@ -502,12 +502,8 @@ bt_page_items(PG_FUNCTION_ARGS) uargs->offset++; SRF_RETURN_NEXT(fctx, result); } - else - { - pfree(uargs->page); - pfree(uargs); - SRF_RETURN_DONE(fctx); - } + + SRF_RETURN_DONE(fctx); } /*------------------------------------------------------- @@ -590,11 +586,8 @@ bt_page_items_bytea(PG_FUNCTION_ARGS) uargs->offset++; SRF_RETURN_NEXT(fctx, result); } - else - { - pfree(uargs); - SRF_RETURN_DONE(fctx); - } + + SRF_RETURN_DONE(fctx); } /* Number of output arguments (columns) for bt_metap() */ diff --git a/contrib/pageinspect/ginfuncs.c b/contrib/pageinspect/ginfuncs.c index 7e2cafab74d..711473579a8 100644 --- a/contrib/pageinspect/ginfuncs.c +++ b/contrib/pageinspect/ginfuncs.c @@ -260,6 +260,6 @@ gin_leafpage_items(PG_FUNCTION_ARGS) SRF_RETURN_NEXT(fctx, result); } - else - SRF_RETURN_DONE(fctx); + + SRF_RETURN_DONE(fctx); } diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c index 984ac331860..3b2f0339cfe 100644 --- a/contrib/pageinspect/hashfuncs.c +++ b/contrib/pageinspect/hashfuncs.c @@ -374,11 +374,8 @@ hash_page_items(PG_FUNCTION_ARGS) SRF_RETURN_NEXT(fctx, result); } - else - { - pfree(uargs); - SRF_RETURN_DONE(fctx); - } + + SRF_RETURN_DONE(fctx); } /* ------------------------------------------------ diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index 50e98caaeb0..fdd0394ffae 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -3388,9 +3388,5 @@ pg_get_multixact_members(PG_FUNCTION_ARGS) SRF_RETURN_NEXT(funccxt, HeapTupleGetDatum(tuple)); } - if (multi->nmembers > 0) - pfree(multi->members); - pfree(multi); - SRF_RETURN_DONE(funccxt); } diff --git a/src/backend/tsearch/wparser.c b/src/backend/tsearch/wparser.c index 88005c0519a..9c1fc7b1014 100644 --- a/src/backend/tsearch/wparser.c +++ b/src/backend/tsearch/wparser.c @@ -104,9 +104,6 @@ tt_process_call(FuncCallContext *funcctx) st->cur++; return result; } - if (st->list) - pfree(st->list); - pfree(st); return (Datum) 0; } @@ -245,12 +242,6 @@ prs_process_call(FuncCallContext *funcctx) st->cur++; return result; } - else - { - if (st->list) - pfree(st->list); - pfree(st); - } return (Datum) 0; } diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index f92861d8d29..4b5007e0d6f 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -535,7 +535,6 @@ jsonb_object_keys(PG_FUNCTION_ARGS) { FuncCallContext *funcctx; OkeysState *state; - int i; if (SRF_IS_FIRSTCALL()) { @@ -598,12 +597,6 @@ jsonb_object_keys(PG_FUNCTION_ARGS) SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt)); } - /* cleanup to reduce or eliminate memory leaks */ - for (i = 0; i < state->result_count; i++) - pfree(state->result[i]); - pfree(state->result); - pfree(state); - SRF_RETURN_DONE(funcctx); } @@ -706,7 +699,6 @@ json_object_keys(PG_FUNCTION_ARGS) { FuncCallContext *funcctx; OkeysState *state; - int i; if (SRF_IS_FIRSTCALL()) { @@ -755,12 +747,6 @@ json_object_keys(PG_FUNCTION_ARGS) SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt)); } - /* cleanup to reduce or eliminate memory leaks */ - for (i = 0; i < state->result_count; i++) - pfree(state->result[i]); - pfree(state->result); - pfree(state); - SRF_RETURN_DONE(funcctx); } diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c index 108dd998c7a..24525879b78 100644 --- a/src/backend/utils/adt/tsvector_op.c +++ b/src/backend/utils/adt/tsvector_op.c @@ -706,7 +706,6 @@ tsvector_unnest(PG_FUNCTION_ARGS) } else { - pfree(tsin); SRF_RETURN_DONE(funcctx); } }