Remove useless pfree()s at the ends of various ValuePerCall SRFs.

tglsfdc · tglsfdc · commit 41b45576d532 · 2020-03-16T21:36:53.000-04:00
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 085b6b6 and b4570d3. 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -706,7 +706,6 @@ tsvector_unnest(PG_FUNCTION_ARGS)
 	}
 	else
 	{
-		pfree(tsin);
 		SRF_RETURN_DONE(funcctx);
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -260,6 +260,6 @@ gin_leafpage_items(PG_FUNCTION_ARGS)`
`260`	`260`
`261`	`261`	`SRF_RETURN_NEXT(fctx, result);`
`262`	`262`	`}`
`263`		`- else`
`264`		`- SRF_RETURN_DONE(fctx);`
	`263`	`+`
	`264`	`+ SRF_RETURN_DONE(fctx);`
`265`	`265`	`}`
Original file line number	Diff line number	Diff line change
`@@ -374,11 +374,8 @@ hash_page_items(PG_FUNCTION_ARGS)`
`374`	`374`
`375`	`375`	`SRF_RETURN_NEXT(fctx, result);`
`376`	`376`	`}`
`377`		`- else`
`378`		`- {`
`379`		`- pfree(uargs);`
`380`		`- SRF_RETURN_DONE(fctx);`
`381`		`- }`
	`377`	`+`
	`378`	`+ SRF_RETURN_DONE(fctx);`
`382`	`379`	`}`
`383`	`380`
`384`	`381`	`/* ------------------------------------------------`
Original file line number	Diff line number	Diff line change
`@@ -3388,9 +3388,5 @@ pg_get_multixact_members(PG_FUNCTION_ARGS)`
`3388`	`3388`	`SRF_RETURN_NEXT(funccxt, HeapTupleGetDatum(tuple));`
`3389`	`3389`	`}`
`3390`	`3390`
`3391`		`- if (multi->nmembers > 0)`
`3392`		`- pfree(multi->members);`
`3393`		`- pfree(multi);`
`3394`		`-`
`3395`	`3391`	`SRF_RETURN_DONE(funccxt);`
`3396`	`3392`	`}`
Original file line number	Diff line number	Diff line change
`@@ -104,9 +104,6 @@ tt_process_call(FuncCallContext *funcctx)`
`104`	`104`	`st->cur++;`
`105`	`105`	`return result;`
`106`	`106`	`}`
`107`		`- if (st->list)`
`108`		`- pfree(st->list);`
`109`		`- pfree(st);`
`110`	`107`	`return (Datum) 0;`
`111`	`108`	`}`
`112`	`109`
`@@ -245,12 +242,6 @@ prs_process_call(FuncCallContext *funcctx)`
`245`	`242`	`st->cur++;`
`246`	`243`	`return result;`
`247`	`244`	`}`
`248`		`- else`
`249`		`- {`
`250`		`- if (st->list)`
`251`		`- pfree(st->list);`
`252`		`- pfree(st);`
`253`		`- }`
`254`	`245`	`return (Datum) 0;`
`255`	`246`	`}`
`256`	`247`
Original file line number	Diff line number	Diff line change
`@@ -535,7 +535,6 @@ jsonb_object_keys(PG_FUNCTION_ARGS)`
`535`	`535`	`{`
`536`	`536`	`FuncCallContext *funcctx;`
`537`	`537`	`OkeysState *state;`
`538`		`- int i;`
`539`	`538`
`540`	`539`	`if (SRF_IS_FIRSTCALL())`
`541`	`540`	`{`
`@@ -598,12 +597,6 @@ jsonb_object_keys(PG_FUNCTION_ARGS)`
`598`	`597`	`SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt));`
`599`	`598`	`}`
`600`	`599`
`601`		`- /* cleanup to reduce or eliminate memory leaks */`
`602`		`- for (i = 0; i < state->result_count; i++)`
`603`		`- pfree(state->result[i]);`
`604`		`- pfree(state->result);`
`605`		`- pfree(state);`
`606`		`-`
`607`	`600`	`SRF_RETURN_DONE(funcctx);`
`608`	`601`	`}`
`609`	`602`
`@@ -706,7 +699,6 @@ json_object_keys(PG_FUNCTION_ARGS)`
`706`	`699`	`{`
`707`	`700`	`FuncCallContext *funcctx;`
`708`	`701`	`OkeysState *state;`
`709`		`- int i;`
`710`	`702`
`711`	`703`	`if (SRF_IS_FIRSTCALL())`
`712`	`704`	`{`
`@@ -755,12 +747,6 @@ json_object_keys(PG_FUNCTION_ARGS)`
`755`	`747`	`SRF_RETURN_NEXT(funcctx, CStringGetTextDatum(nxt));`
`756`	`748`	`}`
`757`	`749`
`758`		`- /* cleanup to reduce or eliminate memory leaks */`
`759`		`- for (i = 0; i < state->result_count; i++)`
`760`		`- pfree(state->result[i]);`
`761`		`- pfree(state->result);`
`762`		`- pfree(state);`
`763`		`-`
`764`	`750`	`SRF_RETURN_DONE(funcctx);`
`765`	`751`	`}`
`766`	`752`
Original file line number	Diff line number	Diff line change
`@@ -706,7 +706,6 @@ tsvector_unnest(PG_FUNCTION_ARGS)`
`706`	`706`	`}`
`707`	`707`	`else`
`708`	`708`	`{`
`709`		`- pfree(tsin);`
`710`	`709`	`SRF_RETURN_DONE(funcctx);`
`711`	`710`	`}`
`712`	`711`	`}`