Change pfree to accept NULL argument

Started by Peter Eisentrautover 3 years ago8 messages
#1Peter Eisentraut
peter.eisentraut@enterprisedb.com
5 attachment(s)

Per discussion in [0]/messages/by-id/1074830.1655442689@sss.pgh.pa.us, here is a patch set that allows pfree() to accept
a NULL argument, like free() does.

Also, a patch that removes the now-unnecessary null pointer checks
before calling pfree(). And a few patches that do the same for some
other functions that I found around. (The one with FreeDir() is perhaps
a bit arguable, since FreeDir() wraps closedir() which does *not* accept
NULL arguments. Also, neither FreeFile() nor the underlying fclose()
accept NULL.)

[0]: /messages/by-id/1074830.1655442689@sss.pgh.pa.us

Attachments:

0001-Remove-unnecessary-casts-in-free-and-pfree.patchtext/plain; charset=UTF-8; name=0001-Remove-unnecessary-casts-in-free-and-pfree.patchDownload
From e80484a9d3a6897984a2bc1f80df55d590e2e85b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Aug 2022 16:51:52 +0200
Subject: [PATCH 1/5] Remove unnecessary casts in free() and pfree()

---
 contrib/sepgsql/label.c        | 8 ++++----
 src/backend/utils/fmgr/dfmgr.c | 6 +++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index e4c98b7eae..6e7c0d7cff 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -662,28 +662,28 @@ quote_object_name(const char *src1, const char *src2,
 		temp = quote_identifier(src1);
 		appendStringInfoString(&result, temp);
 		if (src1 != temp)
-			pfree((void *) temp);
+			pfree(temp);
 	}
 	if (src2)
 	{
 		temp = quote_identifier(src2);
 		appendStringInfo(&result, ".%s", temp);
 		if (src2 != temp)
-			pfree((void *) temp);
+			pfree(temp);
 	}
 	if (src3)
 	{
 		temp = quote_identifier(src3);
 		appendStringInfo(&result, ".%s", temp);
 		if (src3 != temp)
-			pfree((void *) temp);
+			pfree(temp);
 	}
 	if (src4)
 	{
 		temp = quote_identifier(src4);
 		appendStringInfo(&result, ".%s", temp);
 		if (src4 != temp)
-			pfree((void *) temp);
+			pfree(temp);
 	}
 	return result.data;
 }
diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index 08fd7e1264..6ae6fc1556 100644
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -240,7 +240,7 @@ internal_load_library(const char *libname)
 		if (file_scanner->handle == NULL)
 		{
 			load_error = dlerror();
-			free((char *) file_scanner);
+			free(file_scanner);
 			/* errcode_for_file_access might not be appropriate here? */
 			ereport(ERROR,
 					(errcode_for_file_access(),
@@ -263,7 +263,7 @@ internal_load_library(const char *libname)
 
 				/* try to close library */
 				dlclose(file_scanner->handle);
-				free((char *) file_scanner);
+				free(file_scanner);
 
 				/* issue suitable complaint */
 				incompatible_module_error(libname, &module_magic_data);
@@ -273,7 +273,7 @@ internal_load_library(const char *libname)
 		{
 			/* try to close library */
 			dlclose(file_scanner->handle);
-			free((char *) file_scanner);
+			free(file_scanner);
 			/* complain */
 			ereport(ERROR,
 					(errmsg("incompatible library \"%s\": missing magic block",
-- 
2.37.1

0002-Change-pfree-to-accept-NULL-argument.patchtext/plain; charset=UTF-8; name=0002-Change-pfree-to-accept-NULL-argument.patchDownload
From f5eea6d2f1e8a8ed81cf177cef320e041bbff90b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Aug 2022 16:51:52 +0200
Subject: [PATCH 2/5] Change pfree to accept NULL argument

---
 src/backend/utils/mmgr/README | 6 +++++-
 src/backend/utils/mmgr/mcxt.c | 7 ++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/mmgr/README b/src/backend/utils/mmgr/README
index 221b4bd343..e552c6ada6 100644
--- a/src/backend/utils/mmgr/README
+++ b/src/backend/utils/mmgr/README
@@ -66,7 +66,11 @@ pointer, but a valid chunk of which no bytes may be used.  However, the
 chunk might later be repalloc'd larger; it can also be pfree'd without
 error.  Similarly, repalloc allows realloc'ing to zero size.
 
-* pfree and repalloc do not accept a NULL pointer.  This is intentional.
+* repalloc does not accept a NULL pointer.  This is intentional.  (As
+mentioned above, repalloc does not depend on the current memory
+context.  But then it needs to know which memory context to do the
+allocation in.  So the first allocation has to be done outside
+repalloc.)
 
 
 The Current Memory Context
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index e12be1b9bd..d85e961c1e 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -1174,7 +1174,12 @@ palloc_extended(Size size, int flags)
 void
 pfree(void *pointer)
 {
-	MemoryContext context = GetMemoryChunkContext(pointer);
+	MemoryContext context;
+
+	if (!pointer)
+		return;
+
+	context = GetMemoryChunkContext(pointer);
 
 	context->methods->free_p(context, pointer);
 	VALGRIND_MEMPOOL_FREE(context, pointer);
-- 
2.37.1

0003-Remove-no-longer-needed-null-pointer-checks-before-p.patchtext/plain; charset=UTF-8; name=0003-Remove-no-longer-needed-null-pointer-checks-before-p.patchDownload
From 229fcc1ada82c46ffe80ab60779316de4e273831 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Aug 2022 16:51:52 +0200
Subject: [PATCH 3/5] Remove no longer needed null pointer checks before
 pfree()

---
 contrib/bloom/blscan.c                        |  6 +-
 contrib/dblink/dblink.c                       |  9 +-
 contrib/lo/lo.c                               |  6 +-
 contrib/pageinspect/heapfuncs.c               |  6 +-
 .../pg_stat_statements/pg_stat_statements.c   |  6 +-
 contrib/pg_trgm/trgm_gist.c                   |  9 +-
 contrib/pgcrypto/px.c                         |  3 +-
 contrib/postgres_fdw/connection.c             |  3 +-
 contrib/postgres_fdw/postgres_fdw.c           |  3 +-
 contrib/sepgsql/label.c                       |  4 +-
 contrib/sepgsql/uavc.c                        |  6 +-
 contrib/tablefunc/tablefunc.c                 | 10 +--
 contrib/xml2/xpath.c                          |  6 +-
 src/backend/access/common/printtup.c          |  9 +-
 src/backend/access/common/reloptions.c        |  6 +-
 src/backend/access/gin/gindatapage.c          |  3 +-
 src/backend/access/gin/ginget.c               |  3 +-
 src/backend/access/gin/ginscan.c              |  3 +-
 src/backend/access/gin/ginvacuum.c            |  3 +-
 src/backend/access/gist/gistbuild.c           |  3 +-
 src/backend/access/hash/hash.c                |  3 +-
 src/backend/access/heap/heapam.c              | 12 +--
 src/backend/access/heap/vacuumlazy.c          |  3 +-
 src/backend/access/index/genam.c              |  6 +-
 src/backend/access/nbtree/nbtpage.c           | 15 ++--
 src/backend/access/nbtree/nbtree.c            |  9 +-
 src/backend/access/spgist/spgscan.c           | 33 ++-----
 src/backend/access/transam/multixact.c        |  3 +-
 src/backend/access/transam/twophase.c         |  3 +-
 src/backend/access/transam/xact.c             | 15 ++--
 src/backend/access/transam/xlog.c             |  3 +-
 src/backend/access/transam/xlogreader.c       |  6 +-
 src/backend/catalog/dependency.c              |  3 +-
 src/backend/catalog/pg_shdepend.c             |  6 +-
 src/backend/catalog/pg_subscription.c         |  3 +-
 src/backend/commands/analyze.c                |  3 +-
 src/backend/commands/explain.c                |  3 +-
 src/backend/commands/indexcmds.c              |  3 +-
 src/backend/commands/trigger.c                | 12 +--
 src/backend/executor/execTuples.c             | 18 ++--
 src/backend/executor/nodeGather.c             |  3 +-
 src/backend/executor/nodeGatherMerge.c        |  3 +-
 src/backend/executor/nodeTidscan.c            |  3 +-
 src/backend/executor/spi.c                    |  3 +-
 src/backend/executor/tstoreReceiver.c         |  6 +-
 src/backend/jit/llvm/llvmjit.c                |  3 +-
 src/backend/libpq/auth-scram.c                |  6 +-
 src/backend/libpq/auth.c                      |  6 +-
 src/backend/libpq/be-secure-openssl.c         | 14 +--
 src/backend/nodes/bitmapset.c                 |  5 +-
 src/backend/nodes/tidbitmap.c                 |  6 +-
 src/backend/optimizer/plan/planner.c          |  3 +-
 src/backend/parser/scan.l                     |  3 +-
 src/backend/postmaster/autovacuum.c           | 18 ++--
 src/backend/postmaster/checkpointer.c         |  3 +-
 src/backend/postmaster/syslogger.c            |  9 +-
 .../replication/logical/reorderbuffer.c       | 33 +++----
 src/backend/replication/logical/snapbuild.c   | 12 +--
 src/backend/replication/logical/worker.c      |  4 +-
 src/backend/replication/walreceiver.c         |  7 +-
 src/backend/storage/buffer/freelist.c         |  4 +-
 src/backend/storage/ipc/shm_mq.c              |  3 +-
 src/backend/storage/lmgr/lock.c               |  3 +-
 src/backend/tcop/postgres.c                   |  3 +-
 src/backend/tsearch/dict_thesaurus.c          |  9 +-
 src/backend/tsearch/spell.c                   |  6 +-
 src/backend/tsearch/to_tsany.c                |  3 +-
 src/backend/tsearch/wparser_def.c             |  6 +-
 src/backend/utils/activity/pgstat_shmem.c     |  3 +-
 src/backend/utils/adt/jsonb_util.c            |  6 +-
 src/backend/utils/adt/jsonpath_scan.l         |  3 +-
 src/backend/utils/adt/like_support.c          |  6 +-
 src/backend/utils/adt/numeric.c               |  5 +-
 src/backend/utils/adt/pg_locale.c             |  3 +-
 src/backend/utils/adt/tsvector_op.c           |  3 +-
 src/backend/utils/adt/tsvector_parser.c       |  2 +-
 src/backend/utils/adt/varlena.c               |  9 +-
 src/backend/utils/adt/xml.c                   |  4 +-
 src/backend/utils/cache/attoptcache.c         |  3 +-
 src/backend/utils/cache/lsyscache.c           |  9 +-
 src/backend/utils/cache/relcache.c            | 45 +++-------
 src/backend/utils/cache/spccache.c            |  3 +-
 src/backend/utils/cache/ts_cache.c            |  3 +-
 src/backend/utils/cache/typcache.c            |  3 +-
 src/backend/utils/error/elog.c                | 85 ++++++-------------
 src/backend/utils/init/miscinit.c             |  3 +-
 src/backend/utils/mb/stringinfo_mb.c          |  3 +-
 src/backend/utils/misc/guc-file.l             | 21 ++---
 src/backend/utils/misc/guc.c                  |  3 +-
 src/backend/utils/resowner/resowner.c         |  3 +-
 src/backend/utils/sort/logtape.c              |  9 +-
 src/backend/utils/sort/sharedtuplestore.c     |  3 +-
 src/bin/pg_verifybackup/parse_manifest.c      | 42 +++------
 src/common/compression.c                      |  3 +-
 src/include/replication/walreceiver.h         |  3 +-
 src/pl/plperl/plperl.c                        |  3 +-
 src/pl/plpython/plpy_elog.c                   |  6 +-
 src/pl/plpython/plpy_exec.c                   |  9 +-
 src/pl/plpython/plpy_plpymodule.c             |  3 +-
 src/pl/plpython/plpy_typeio.c                 |  6 +-
 .../modules/test_oat_hooks/test_oat_hooks.c   |  6 +-
 101 files changed, 250 insertions(+), 527 deletions(-)

diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index 4c923488bd..945d37d316 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -50,8 +50,7 @@ blrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
 {
 	BloomScanOpaque so = (BloomScanOpaque) scan->opaque;
 
-	if (so->sign)
-		pfree(so->sign);
+	pfree(so->sign);
 	so->sign = NULL;
 
 	if (scankey && scan->numberOfKeys > 0)
@@ -69,8 +68,7 @@ blendscan(IndexScanDesc scan)
 {
 	BloomScanOpaque so = (BloomScanOpaque) scan->opaque;
 
-	if (so->sign)
-		pfree(so->sign);
+	pfree(so->sign);
 	so->sign = NULL;
 }
 
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index e323fdd0e6..69fce2a3e9 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -342,8 +342,7 @@ dblink_connect(PG_FUNCTION_ARGS)
 		msg = pchomp(PQerrorMessage(conn));
 		PQfinish(conn);
 		ReleaseExternalFD();
-		if (rconn)
-			pfree(rconn);
+		pfree(rconn);
 
 		ereport(ERROR,
 				(errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION),
@@ -1274,8 +1273,7 @@ storeRow(volatile storeInfo *sinfo, PGresult *res, bool first)
 		 * Set up sufficiently-wide string pointers array; this won't change
 		 * in size so it's easy to preallocate.
 		 */
-		if (sinfo->cstrs)
-			pfree(sinfo->cstrs);
+		pfree(sinfo->cstrs);
 		sinfo->cstrs = (char **) palloc(nfields * sizeof(char *));
 	}
 
@@ -2644,8 +2642,7 @@ dblink_security_check(PGconn *conn, remoteConn *rconn)
 		{
 			PQfinish(conn);
 			ReleaseExternalFD();
-			if (rconn)
-				pfree(rconn);
+			pfree(rconn);
 
 			ereport(ERROR,
 					(errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED),
diff --git a/contrib/lo/lo.c b/contrib/lo/lo.c
index 457be26c4e..46909ead07 100644
--- a/contrib/lo/lo.c
+++ b/contrib/lo/lo.c
@@ -83,10 +83,8 @@ lo_manage(PG_FUNCTION_ARGS)
 			DirectFunctionCall1(be_lo_unlink,
 								ObjectIdGetDatum(atooid(orig)));
 
-		if (newv)
-			pfree(newv);
-		if (orig)
-			pfree(orig);
+		pfree(newv);
+		pfree(orig);
 	}
 
 	/*
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 2ff70405cf..fb356ffd7c 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -397,8 +397,7 @@ tuple_data_split_internal(Oid relid, char *tupdata,
 
 		raw_attrs = accumArrayResult(raw_attrs, PointerGetDatum(attr_data),
 									 is_null, BYTEAOID, CurrentMemoryContext);
-		if (attr_data)
-			pfree(attr_data);
+		pfree(attr_data);
 	}
 
 	if (tupdata_len != off)
@@ -489,8 +488,7 @@ tuple_data_split(PG_FUNCTION_ARGS)
 									t_infomask, t_infomask2, t_bits,
 									do_detoast);
 
-	if (t_bits)
-		pfree(t_bits);
+	pfree(t_bits);
 
 	PG_RETURN_ARRAYTYPE_P(res);
 }
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index ba868f0de9..6c08ae1bde 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -697,8 +697,7 @@ pgss_shmem_startup(void)
 			 errmsg("could not write file \"%s\": %m",
 					PGSS_TEXT_FILE)));
 fail:
-	if (buffer)
-		pfree(buffer);
+	pfree(buffer);
 	if (file)
 		FreeFile(file);
 	if (qfile)
@@ -1407,8 +1406,7 @@ pgss_store(const char *query, uint64 queryId,
 	LWLockRelease(pgss->lock);
 
 	/* We postpone this clean-up until we're out of the lock */
-	if (norm_query)
-		pfree(norm_query);
+	pfree(norm_query);
 }
 
 /*
diff --git a/contrib/pg_trgm/trgm_gist.c b/contrib/pg_trgm/trgm_gist.c
index 7a7be807af..457f13226b 100644
--- a/contrib/pg_trgm/trgm_gist.c
+++ b/contrib/pg_trgm/trgm_gist.c
@@ -290,8 +290,7 @@ gtrgm_consistent(PG_FUNCTION_ARGS)
 			newcache->trigrams = NULL;
 		newcache->graph = graph;
 
-		if (cache)
-			pfree(cache);
+		pfree(cache);
 		fcinfo->flinfo->fn_extra = (void *) newcache;
 		cache = newcache;
 	}
@@ -475,8 +474,7 @@ gtrgm_distance(PG_FUNCTION_ARGS)
 		memcpy(newcache, query, querysize);
 		memcpy(newcache + MAXALIGN(querysize), qtrg, VARSIZE(qtrg));
 
-		if (cache)
-			pfree(cache);
+		pfree(cache);
 		fcinfo->flinfo->fn_extra = newcache;
 		cache = newcache;
 	}
@@ -714,8 +712,7 @@ gtrgm_penalty(PG_FUNCTION_ARGS)
 			cachedVal = (TRGM *) (newcache + MAXALIGN(siglen));
 			memcpy(cachedVal, newval, newvalsize);
 
-			if (cache)
-				pfree(cache);
+			pfree(cache);
 			fcinfo->flinfo->fn_extra = newcache;
 			cache = newcache;
 		}
diff --git a/contrib/pgcrypto/px.c b/contrib/pgcrypto/px.c
index 3b098c6151..53d1aceda6 100644
--- a/contrib/pgcrypto/px.c
+++ b/contrib/pgcrypto/px.c
@@ -208,8 +208,7 @@ combo_init(PX_Combo *cx, const uint8 *key, unsigned klen,
 
 	err = px_cipher_init(c, keybuf, klen, ivbuf);
 
-	if (ivbuf)
-		pfree(ivbuf);
+	pfree(ivbuf);
 	pfree(keybuf);
 
 	return err;
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 939d114f02..cf13049a7b 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -500,8 +500,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user)
 		/* Prepare new session for use */
 		configure_remote_session(conn);
 
-		if (appname != NULL)
-			pfree(appname);
+		pfree(appname);
 		pfree(keywords);
 		pfree(values);
 	}
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 16320170ce..1c9c771a28 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -6032,8 +6032,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
 	if (!foreign_join_ok(root, joinrel, jointype, outerrel, innerrel, extra))
 	{
 		/* Free path required for EPQ if we copied one; we don't need it now */
-		if (epq_path)
-			pfree(epq_path);
+		pfree(epq_path);
 		return;
 	}
 
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index 6e7c0d7cff..e9476c9322 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -177,9 +177,7 @@ sepgsql_xact_callback(XactEvent event, void *arg)
 			else
 				new_label = NULL;
 
-			if (client_label_committed)
-				pfree(client_label_committed);
-
+			pfree(client_label_committed);
 			client_label_committed = new_label;
 
 			/*
diff --git a/contrib/sepgsql/uavc.c b/contrib/sepgsql/uavc.c
index 76597983f4..042a51dbfd 100644
--- a/contrib/sepgsql/uavc.c
+++ b/contrib/sepgsql/uavc.c
@@ -109,8 +109,7 @@ sepgsql_avc_reclaim(void)
 
 				pfree(cache->scontext);
 				pfree(cache->tcontext);
-				if (cache->ncontext)
-					pfree(cache->ncontext);
+				pfree(cache->ncontext);
 				pfree(cache);
 
 				avc_num_caches--;
@@ -428,8 +427,7 @@ sepgsql_avc_check_perms(const ObjectAddress *tobject,
 	rc = sepgsql_avc_check_perms_label(tcontext,
 									   tclass, required,
 									   audit_name, abort_on_violation);
-	if (tcontext)
-		pfree(tcontext);
+	pfree(tcontext);
 
 	return rc;
 }
diff --git a/contrib/tablefunc/tablefunc.c b/contrib/tablefunc/tablefunc.c
index e308228bde..8d33510cee 100644
--- a/contrib/tablefunc/tablefunc.c
+++ b/contrib/tablefunc/tablefunc.c
@@ -93,11 +93,8 @@ typedef struct
 
 #define xpfree(var_) \
 	do { \
-		if (var_ != NULL) \
-		{ \
-			pfree(var_); \
-			var_ = NULL; \
-		} \
+		pfree(var_); \
+		var_ = NULL; \
 	} while (0)
 
 #define xpstrdup(tgtvar_, srcvar_) \
@@ -584,8 +581,7 @@ crosstab(PG_FUNCTION_ARGS)
 
 		/* Clean up */
 		for (i = 0; i < num_categories + 1; i++)
-			if (values[i] != NULL)
-				pfree(values[i]);
+			pfree(values[i]);
 		pfree(values);
 	}
 
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index b8ee757674..b9297967bf 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -711,10 +711,8 @@ xpath_table(PG_FUNCTION_ARGS)
 				xmlFreeDoc(doctree);
 			doctree = NULL;
 
-			if (pkey)
-				pfree(pkey);
-			if (xmldoc)
-				pfree(xmldoc);
+			pfree(pkey);
+			pfree(xmldoc);
 		}
 	}
 	PG_CATCH();
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index d2f3b57288..a1736d1010 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -253,8 +253,7 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs)
 	int			i;
 
 	/* get rid of any old data */
-	if (myState->myinfo)
-		pfree(myState->myinfo);
+	pfree(myState->myinfo);
 	myState->myinfo = NULL;
 
 	myState->attrinfo = typeinfo;
@@ -387,14 +386,12 @@ printtup_shutdown(DestReceiver *self)
 {
 	DR_printtup *myState = (DR_printtup *) self;
 
-	if (myState->myinfo)
-		pfree(myState->myinfo);
+	pfree(myState->myinfo);
 	myState->myinfo = NULL;
 
 	myState->attrinfo = NULL;
 
-	if (myState->buf.data)
-		pfree(myState->buf.data);
+	pfree(myState->buf.data);
 	myState->buf.data = NULL;
 
 	if (myState->tmpcontext)
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 609329bb21..2e56dedad9 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -617,8 +617,7 @@ initialize_reloptions(void)
 	}
 	j += num_custom_options;
 
-	if (relOpts)
-		pfree(relOpts);
+	pfree(relOpts);
 	relOpts = MemoryContextAlloc(TopMemoryContext,
 								 (j + 1) * sizeof(relopt_gen *));
 
@@ -1972,8 +1971,7 @@ build_local_reloptions(local_relopts *relopts, Datum options, bool validate)
 	foreach(lc, relopts->validators)
 		((relopts_validator) lfirst(lc)) (opts, vals, noptions);
 
-	if (elems)
-		pfree(elems);
+	pfree(elems);
 
 	return opts;
 }
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
index 7c76d1f90d..41783d2e38 100644
--- a/src/backend/access/gin/gindatapage.c
+++ b/src/backend/access/gin/gindatapage.c
@@ -1614,8 +1614,7 @@ leafRepackItems(disassembledLeaf *leaf, ItemPointer remaining)
 					 * The new segment is inserted after this one, so it will
 					 * be processed in the next iteration of this loop.
 					 */
-					if (seginfo->seg)
-						pfree(seginfo->seg);
+					pfree(seginfo->seg);
 					seginfo->seg = ginCompressPostingList(seginfo->items,
 														  seginfo->nitems,
 														  GinPostingListSegmentTargetSize,
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index fc85ba99ac..78f4df1bf8 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -327,8 +327,7 @@ startScanEntry(GinState *ginstate, GinScanEntry entry, Snapshot snapshot)
 	entry->buffer = InvalidBuffer;
 	ItemPointerSetMin(&entry->curItem);
 	entry->offset = InvalidOffsetNumber;
-	if (entry->list)
-		pfree(entry->list);
+	pfree(entry->list);
 	entry->list = NULL;
 	entry->nlist = 0;
 	entry->matchBitmap = NULL;
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index b776d04459..3ffbc82b29 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -243,8 +243,7 @@ ginFreeScanKeys(GinScanOpaque so)
 
 		if (entry->buffer != InvalidBuffer)
 			ReleaseBuffer(entry->buffer);
-		if (entry->list)
-			pfree(entry->list);
+		pfree(entry->list);
 		if (entry->matchIterator)
 			tbm_end_iterate(entry->matchIterator);
 		if (entry->matchBitmap)
diff --git a/src/backend/access/gin/ginvacuum.c b/src/backend/access/gin/ginvacuum.c
index b4fa5f6bf8..fab8e954c9 100644
--- a/src/backend/access/gin/ginvacuum.c
+++ b/src/backend/access/gin/ginvacuum.c
@@ -544,8 +544,7 @@ ginVacuumEntryPage(GinVacuumState *gvs, Buffer buffer, BlockNumber *roots, uint3
 				itup = GinFormTuple(&gvs->ginstate, attnum, key, category,
 									(char *) plist, plistsize,
 									nitems, true);
-				if (plist)
-					pfree(plist);
+				pfree(plist);
 				PageIndexTupleDelete(tmppage, i);
 
 				if (PageAddItem(tmppage, (Item) itup, IndexTupleSize(itup), i, false, false) != i)
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index 374e64e808..639c312f40 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -448,8 +448,7 @@ gist_indexsortbuild(GISTBuildState *state)
 		gist_indexsortbuild_levelstate_flush(state, levelstate);
 		parent = levelstate->parent;
 		for (int i = 0; i < GIST_SORTED_BUILD_PAGE_NUM; i++)
-			if (levelstate->pages[i])
-				pfree(levelstate->pages[i]);
+			pfree(levelstate->pages[i]);
 		pfree(levelstate);
 		levelstate = parent;
 	}
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index c361509d68..e5cd9f08d9 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -439,8 +439,7 @@ hashendscan(IndexScanDesc scan)
 
 	_hash_dropscanbuf(rel, so);
 
-	if (so->killedItems != NULL)
-		pfree(so->killedItems);
+	pfree(so->killedItems);
 	pfree(so);
 	scan->opaque = NULL;
 }
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index aab8d6fa4e..e79c334b81 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1277,14 +1277,12 @@ heap_endscan(TableScanDesc sscan)
 	 */
 	RelationDecrementReferenceCount(scan->rs_base.rs_rd);
 
-	if (scan->rs_base.rs_key)
-		pfree(scan->rs_base.rs_key);
+	pfree(scan->rs_base.rs_key);
 
 	if (scan->rs_strategy != NULL)
 		FreeAccessStrategy(scan->rs_strategy);
 
-	if (scan->rs_parallelworkerdata != NULL)
-		pfree(scan->rs_parallelworkerdata);
+	pfree(scan->rs_parallelworkerdata);
 
 	if (scan->rs_base.rs_flags & SO_TEMP_SNAPSHOT)
 		UnregisterSnapshot(scan->rs_base.rs_snapshot);
@@ -4377,8 +4375,7 @@ heap_lock_tuple(Relation relation, HeapTuple tuple,
 					}
 				}
 
-				if (members)
-					pfree(members);
+				pfree(members);
 			}
 			else if (TransactionIdIsCurrentTransactionId(xwait))
 			{
@@ -5536,8 +5533,7 @@ heap_lock_updated_tuple_rec(Relation rel, ItemPointer tid, TransactionId xid,
 						goto out_locked;
 					}
 				}
-				if (members)
-					pfree(members);
+				pfree(members);
 			}
 			else
 			{
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index b802ed247e..65f8e0db27 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -797,8 +797,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
 	/* Cleanup index statistics and index names */
 	for (int i = 0; i < vacrel->nindexes; i++)
 	{
-		if (vacrel->indstats[i])
-			pfree(vacrel->indstats[i]);
+		pfree(vacrel->indstats[i]);
 
 		if (instrument)
 			pfree(indnames[i]);
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index 98af5347b9..8f7f5728a7 100644
--- a/src/backend/access/index/genam.c
+++ b/src/backend/access/index/genam.c
@@ -144,10 +144,8 @@ RelationGetIndexScan(Relation indexRelation, int nkeys, int norderbys)
 void
 IndexScanEnd(IndexScanDesc scan)
 {
-	if (scan->keyData != NULL)
-		pfree(scan->keyData);
-	if (scan->orderByData != NULL)
-		pfree(scan->orderByData);
+	pfree(scan->keyData);
+	pfree(scan->orderByData);
 
 	pfree(scan);
 }
diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c
index 8b96708b3e..66cca869d5 100644
--- a/src/backend/access/nbtree/nbtpage.c
+++ b/src/backend/access/nbtree/nbtpage.c
@@ -391,8 +391,7 @@ _bt_getroot(Relation rel, int access)
 		}
 		_bt_relbuf(rel, rootbuf);
 		/* Cache is stale, throw it away */
-		if (rel->rd_amcache)
-			pfree(rel->rd_amcache);
+		pfree(rel->rd_amcache);
 		rel->rd_amcache = NULL;
 	}
 
@@ -592,8 +591,7 @@ _bt_gettrueroot(Relation rel)
 	 * is out-of-date anyway.  In light of point (b), it's probably safest to
 	 * actively flush any cached metapage info.
 	 */
-	if (rel->rd_amcache)
-		pfree(rel->rd_amcache);
+	pfree(rel->rd_amcache);
 	rel->rd_amcache = NULL;
 
 	metabuf = _bt_getbuf(rel, BTREE_METAPAGE, BT_READ);
@@ -1267,8 +1265,7 @@ _bt_delitems_vacuum(Relation rel, Buffer buf,
 	END_CRIT_SECTION();
 
 	/* can't leak memory here */
-	if (updatedbuf != NULL)
-		pfree(updatedbuf);
+	pfree(updatedbuf);
 	/* free tuples allocated within _bt_delitems_update() */
 	for (int i = 0; i < nupdatable; i++)
 		pfree(updatable[i]->itup);
@@ -1384,8 +1381,7 @@ _bt_delitems_delete(Relation rel, Buffer buf, TransactionId latestRemovedXid,
 	END_CRIT_SECTION();
 
 	/* can't leak memory here */
-	if (updatedbuf != NULL)
-		pfree(updatedbuf);
+	pfree(updatedbuf);
 	/* free tuples allocated within _bt_delitems_update() */
 	for (int i = 0; i < nupdatable; i++)
 		pfree(updatable[i]->itup);
@@ -2961,8 +2957,7 @@ _bt_pendingfsm_finalize(Relation rel, BTVacState *vstate)
 	if (vstate->npendingpages == 0)
 	{
 		/* Just free memory when nothing to do */
-		if (vstate->pendingpages)
-			pfree(vstate->pendingpages);
+		pfree(vstate->pendingpages);
 
 		return;
 	}
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index b52eca8f38..6e1604c799 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -464,15 +464,12 @@ btendscan(IndexScanDesc scan)
 	/* No need to invalidate positions, the RAM is about to be freed. */
 
 	/* Release storage */
-	if (so->keyData != NULL)
-		pfree(so->keyData);
+	pfree(so->keyData);
 	/* so->arrayKeyData and so->arrayKeys are in arrayContext */
 	if (so->arrayContext != NULL)
 		MemoryContextDelete(so->arrayContext);
-	if (so->killedItems != NULL)
-		pfree(so->killedItems);
-	if (so->currTuples != NULL)
-		pfree(so->currTuples);
+	pfree(so->killedItems);
+	pfree(so->currTuples);
 	/* so->markTuples should not be pfree'd, see btrescan */
 	pfree(so);
 }
diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c
index 87a345d290..7ccdba37db 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -90,11 +90,8 @@ spgFreeSearchItem(SpGistScanOpaque so, SpGistSearchItem *item)
 		DatumGetPointer(item->value) != NULL)
 		pfree(DatumGetPointer(item->value));
 
-	if (item->leafTuple)
-		pfree(item->leafTuple);
-
-	if (item->traversalValue)
-		pfree(item->traversalValue);
+	pfree(item->leafTuple);
+	pfree(item->traversalValue);
 
 	pfree(item);
 }
@@ -175,19 +172,14 @@ resetSpGistScanOpaque(SpGistScanOpaque so)
 	if (so->numberOfOrderBys > 0)
 	{
 		/* Must pfree distances to avoid memory leak */
-		int			i;
-
-		for (i = 0; i < so->nPtrs; i++)
-			if (so->distances[i])
-				pfree(so->distances[i]);
+		for (int i = 0; i < so->nPtrs; i++)
+			pfree(so->distances[i]);
 	}
 
 	if (so->want_itup)
 	{
 		/* Must pfree reconstructed tuples to avoid memory leak */
-		int			i;
-
-		for (i = 0; i < so->nPtrs; i++)
+		for (int i = 0; i < so->nPtrs; i++)
 			pfree(so->reconTups[i]);
 	}
 	so->iPtr = so->nPtrs = 0;
@@ -433,15 +425,13 @@ spgendscan(IndexScanDesc scan)
 	MemoryContextDelete(so->tempCxt);
 	MemoryContextDelete(so->traversalCxt);
 
-	if (so->keyData)
-		pfree(so->keyData);
+	pfree(so->keyData);
 
 	if (so->state.leafTupDesc &&
 		so->state.leafTupDesc != RelationGetDescr(so->state.index))
 		FreeTupleDesc(so->state.leafTupDesc);
 
-	if (so->state.deadTupleStorage)
-		pfree(so->state.deadTupleStorage);
+	pfree(so->state.deadTupleStorage);
 
 	if (scan->numberOfOrderBys > 0)
 	{
@@ -1054,19 +1044,14 @@ spggettuple(IndexScanDesc scan, ScanDirection dir)
 		if (so->numberOfOrderBys > 0)
 		{
 			/* Must pfree distances to avoid memory leak */
-			int			i;
-
-			for (i = 0; i < so->nPtrs; i++)
-				if (so->distances[i])
+			for (int i = 0; i < so->nPtrs; i++)
 					pfree(so->distances[i]);
 		}
 
 		if (so->want_itup)
 		{
 			/* Must pfree reconstructed tuples to avoid memory leak */
-			int			i;
-
-			for (i = 0; i < so->nPtrs; i++)
+			for (int i = 0; i < so->nPtrs; i++)
 				pfree(so->reconTups[i]);
 		}
 		so->iPtr = so->nPtrs = 0;
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 8f7d12950e..bdd1a7dc40 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -1658,8 +1658,7 @@ mxid_to_string(MultiXactId multi, int nmembers, MultiXactMember *members)
 	StringInfoData buf;
 	int			i;
 
-	if (str != NULL)
-		pfree(str);
+	pfree(str);
 
 	initStringInfo(&buf);
 
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 803d169f57..75edd8ec2b 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -2033,8 +2033,7 @@ StandbyRecoverPreparedTransactions(void)
 		buf = ProcessTwoPhaseBuffer(xid,
 									gxact->prepare_start_lsn,
 									gxact->ondisk, false, false);
-		if (buf != NULL)
-			pfree(buf);
+		pfree(buf);
 	}
 	LWLockRelease(TwoPhaseStateLock);
 }
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 50f092d7eb..c8a5b4610c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1522,8 +1522,7 @@ RecordTransactionCommit(void)
 	XactLastRecEnd = 0;
 cleanup:
 	/* Clean up local data */
-	if (rels)
-		pfree(rels);
+	pfree(rels);
 	if (ndroppedstats)
 		pfree(droppedstats);
 
@@ -1680,8 +1679,7 @@ AtSubCommit_childXids(void)
 	s->parent->nChildXids = new_nChildXids;
 
 	/* Release child's array to avoid leakage */
-	if (s->childXids != NULL)
-		pfree(s->childXids);
+	pfree(s->childXids);
 	/* We must reset these to avoid double-free if fail later in commit */
 	s->childXids = NULL;
 	s->nChildXids = 0;
@@ -1806,8 +1804,7 @@ RecordTransactionAbort(bool isSubXact)
 		XactLastRecEnd = 0;
 
 	/* And clean up local data */
-	if (rels)
-		pfree(rels);
+	pfree(rels);
 	if (ndroppedstats)
 		pfree(droppedstats);
 
@@ -1885,8 +1882,7 @@ AtSubAbort_childXids(void)
 	 * AtSubCommit_childXids).  This means we'd better free the array
 	 * explicitly at abort to avoid leakage.
 	 */
-	if (s->childXids != NULL)
-		pfree(s->childXids);
+	pfree(s->childXids);
 	s->childXids = NULL;
 	s->nChildXids = 0;
 	s->maxChildXids = 0;
@@ -5291,8 +5287,7 @@ PopTransaction(void)
 	CurrentResourceOwner = s->parent->curTransactionOwner;
 
 	/* Free the old child structure */
-	if (s->name)
-		pfree(s->name);
+	pfree(s->name);
 	pfree(s);
 }
 
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 87b243e0d4..6c876a6e18 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4419,8 +4419,7 @@ XLOGShmemInit(void)
 		/* Initialize local copy of WALInsertLocks */
 		WALInsertLocks = XLogCtl->Insert.WALInsertLocks;
 
-		if (localControlFile)
-			pfree(localControlFile);
+		pfree(localControlFile);
 		return;
 	}
 	memset(XLogCtl, 0, sizeof(XLogCtlData));
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index f17e80948d..0b5f2768fc 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -176,8 +176,7 @@ XLogReaderFree(XLogReaderState *state)
 		pfree(state->decode_buffer);
 
 	pfree(state->errormsg_buf);
-	if (state->readRecordBuf)
-		pfree(state->readRecordBuf);
+	pfree(state->readRecordBuf);
 	pfree(state->readBuf);
 	pfree(state);
 }
@@ -220,8 +219,7 @@ allocate_recordbuf(XLogReaderState *state, uint32 reclength)
 
 #endif
 
-	if (state->readRecordBuf)
-		pfree(state->readRecordBuf);
+	pfree(state->readRecordBuf);
 	state->readRecordBuf =
 		(char *) palloc_extended(newSize, MCXT_ALLOC_NO_OOM);
 	if (state->readRecordBuf == NULL)
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 39768fa22b..e33d34a4a3 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2821,8 +2821,7 @@ void
 free_object_addresses(ObjectAddresses *addrs)
 {
 	pfree(addrs->refs);
-	if (addrs->extras)
-		pfree(addrs->extras);
+	pfree(addrs->extras);
 	pfree(addrs);
 }
 
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index f2f227f887..111e689d4c 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -543,10 +543,8 @@ updateAclDependencies(Oid classId, Oid objectId, int32 objsubId,
 		table_close(sdepRel, RowExclusiveLock);
 	}
 
-	if (oldmembers)
-		pfree(oldmembers);
-	if (newmembers)
-		pfree(newmembers);
+	pfree(oldmembers);
+	pfree(newmembers);
 }
 
 /*
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index a506fc3ec8..1fb333ccf5 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -160,8 +160,7 @@ FreeSubscription(Subscription *sub)
 {
 	pfree(sub->name);
 	pfree(sub->conninfo);
-	if (sub->slotname)
-		pfree(sub->slotname);
+	pfree(sub->slotname);
 	list_free_deep(sub->publications);
 	pfree(sub);
 }
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index a7966fff83..6663e70861 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -717,8 +717,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
 
 			stats = index_vacuum_cleanup(&ivinfo, NULL);
 
-			if (stats)
-				pfree(stats);
+			pfree(stats);
 		}
 	}
 
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e078456b19..88d05a6edb 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -1061,8 +1061,7 @@ report_triggers(ResultRelInfo *rInfo, bool show_relname, ExplainState *es)
 			ExplainPropertyFloat("Calls", NULL, instr->ntuples, 0, es);
 		}
 
-		if (conname)
-			pfree(conname);
+		pfree(conname);
 
 		ExplainCloseGroup("Trigger", NULL, true, es);
 	}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 667f2a4cd1..605367d20d 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -301,8 +301,7 @@ CheckIndexCompatible(Oid oldId,
 		ret = CompareOpclassOptions(opclassOptions,
 									indexInfo->ii_OpclassOptions, old_natts);
 
-		if (opclassOptions)
-			pfree(opclassOptions);
+		pfree(opclassOptions);
 	}
 
 	/* Any change in exclusion operator selections breaks compatibility. */
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 62a09fb131..349f297b7c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2194,12 +2194,9 @@ FreeTriggerDesc(TriggerDesc *trigdesc)
 				pfree(trigger->tgargs[trigger->tgnargs]);
 			pfree(trigger->tgargs);
 		}
-		if (trigger->tgqual)
-			pfree(trigger->tgqual);
-		if (trigger->tgoldtable)
-			pfree(trigger->tgoldtable);
-		if (trigger->tgnewtable)
-			pfree(trigger->tgnewtable);
+		pfree(trigger->tgqual);
+		pfree(trigger->tgoldtable);
+		pfree(trigger->tgnewtable);
 		trigger++;
 	}
 	pfree(trigdesc->triggers);
@@ -5323,8 +5320,7 @@ AfterTriggerEndSubXact(bool isCommit)
 		Assert(my_level < afterTriggers.maxtransdepth);
 		/* If we saved a prior state, we don't need it anymore */
 		state = afterTriggers.trans_stack[my_level].state;
-		if (state != NULL)
-			pfree(state);
+		pfree(state);
 		/* this avoids double pfree if error later: */
 		afterTriggers.trans_stack[my_level].state = NULL;
 		Assert(afterTriggers.query_depth ==
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 06ac253ea0..6f1ccd424e 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -1211,10 +1211,8 @@ ExecResetTupleTable(List *tupleTable,	/* tuple table */
 		{
 			if (!TTS_FIXED(slot))
 			{
-				if (slot->tts_values)
-					pfree(slot->tts_values);
-				if (slot->tts_isnull)
-					pfree(slot->tts_isnull);
+				pfree(slot->tts_values);
+				pfree(slot->tts_isnull);
 			}
 			pfree(slot);
 		}
@@ -1261,10 +1259,8 @@ ExecDropSingleTupleTableSlot(TupleTableSlot *slot)
 		ReleaseTupleDesc(slot->tts_tupleDescriptor);
 	if (!TTS_FIXED(slot))
 	{
-		if (slot->tts_values)
-			pfree(slot->tts_values);
-		if (slot->tts_isnull)
-			pfree(slot->tts_isnull);
+		pfree(slot->tts_values);
+		pfree(slot->tts_isnull);
 	}
 	pfree(slot);
 }
@@ -1301,10 +1297,8 @@ ExecSetSlotDescriptor(TupleTableSlot *slot, /* slot to change */
 	if (slot->tts_tupleDescriptor)
 		ReleaseTupleDesc(slot->tts_tupleDescriptor);
 
-	if (slot->tts_values)
-		pfree(slot->tts_values);
-	if (slot->tts_isnull)
-		pfree(slot->tts_isnull);
+	pfree(slot->tts_values);
+	pfree(slot->tts_isnull);
 
 	/*
 	 * Install the new descriptor; if it's refcounted, bump its refcount.
diff --git a/src/backend/executor/nodeGather.c b/src/backend/executor/nodeGather.c
index 1283d5b737..d984c15b22 100644
--- a/src/backend/executor/nodeGather.c
+++ b/src/backend/executor/nodeGather.c
@@ -404,8 +404,7 @@ ExecShutdownGatherWorkers(GatherState *node)
 		ExecParallelFinish(node->pei);
 
 	/* Flush local copy of reader array */
-	if (node->reader)
-		pfree(node->reader);
+	pfree(node->reader);
 	node->reader = NULL;
 }
 
diff --git a/src/backend/executor/nodeGatherMerge.c b/src/backend/executor/nodeGatherMerge.c
index 3b1007f352..0e518a348d 100644
--- a/src/backend/executor/nodeGatherMerge.c
+++ b/src/backend/executor/nodeGatherMerge.c
@@ -327,8 +327,7 @@ ExecShutdownGatherMergeWorkers(GatherMergeState *node)
 		ExecParallelFinish(node->pei);
 
 	/* Flush local copy of reader array */
-	if (node->reader)
-		pfree(node->reader);
+	pfree(node->reader);
 	node->reader = NULL;
 }
 
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
index a1c6325d64..c370fb9f79 100644
--- a/src/backend/executor/nodeTidscan.c
+++ b/src/backend/executor/nodeTidscan.c
@@ -442,8 +442,7 @@ ExecTidScan(PlanState *pstate)
 void
 ExecReScanTidScan(TidScanState *node)
 {
-	if (node->tss_TidList)
-		pfree(node->tss_TidList);
+	pfree(node->tss_TidList);
 	node->tss_TidList = NULL;
 	node->tss_NumTids = 0;
 	node->tss_TidPtr = -1;
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 29bc26669b..ac5e145e39 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1454,8 +1454,7 @@ SPI_cursor_open(const char *name, SPIPlanPtr plan,
 	portal = SPI_cursor_open_internal(name, plan, paramLI, read_only);
 
 	/* done with the transient ParamListInfo */
-	if (paramLI)
-		pfree(paramLI);
+	pfree(paramLI);
 
 	return portal;
 }
diff --git a/src/backend/executor/tstoreReceiver.c b/src/backend/executor/tstoreReceiver.c
index 0a19904b1f..8260a131a0 100644
--- a/src/backend/executor/tstoreReceiver.c
+++ b/src/backend/executor/tstoreReceiver.c
@@ -208,11 +208,9 @@ tstoreShutdownReceiver(DestReceiver *self)
 	TStoreState *myState = (TStoreState *) self;
 
 	/* Release workspace if any */
-	if (myState->outvalues)
-		pfree(myState->outvalues);
+	pfree(myState->outvalues);
 	myState->outvalues = NULL;
-	if (myState->tofree)
-		pfree(myState->tofree);
+	pfree(myState->tofree);
 	myState->tofree = NULL;
 	if (myState->tupmap)
 		free_conversion_map(myState->tupmap);
diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c
index 6c72d43beb..86362b0351 100644
--- a/src/backend/jit/llvm/llvmjit.c
+++ b/src/backend/jit/llvm/llvmjit.c
@@ -1094,8 +1094,7 @@ llvm_resolve_symbol(const char *symname, void *ctx)
 		addr = (uintptr_t) LLVMSearchForAddressOfSymbol(symname);
 
 	pfree(funcname);
-	if (modname)
-		pfree(modname);
+	pfree(modname);
 
 	/* let LLVM will error out - should never happen */
 	if (!addr)
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index ee7f52218a..a01e707aa5 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -486,8 +486,7 @@ pg_be_scram_build_secret(const char *password)
 								SCRAM_DEFAULT_ITERATIONS, password,
 								&errstr);
 
-	if (prep_password)
-		pfree(prep_password);
+	pfree(prep_password);
 
 	return result;
 }
@@ -548,8 +547,7 @@ scram_verify_plain_password(const char *username, const char *password,
 		elog(ERROR, "could not compute server key: %s", errstr);
 	}
 
-	if (prep_password)
-		pfree(prep_password);
+	pfree(prep_password);
 
 	/*
 	 * Compare the secret's Server Key with the one computed from the
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 1545ff9f16..6495c9ce6b 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -791,8 +791,7 @@ CheckPasswordAuth(Port *port, const char **logdetail)
 	else
 		result = STATUS_ERROR;
 
-	if (shadow_pass)
-		pfree(shadow_pass);
+	pfree(shadow_pass);
 	pfree(passwd);
 
 	if (result == STATUS_OK)
@@ -847,8 +846,7 @@ CheckPWChallengeAuth(Port *port, const char **logdetail)
 		auth_result = CheckSASLAuth(&pg_be_scram_mech, port, shadow_pass,
 									logdetail);
 
-	if (shadow_pass)
-		pfree(shadow_pass);
+	pfree(shadow_pass);
 
 	/*
 	 * If get_role_password() returned error, return error, even if the
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 55d4b29f7e..d0f3478a08 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -679,17 +679,11 @@ be_tls_close(Port *port)
 		port->peer = NULL;
 	}
 
-	if (port->peer_cn)
-	{
-		pfree(port->peer_cn);
-		port->peer_cn = NULL;
-	}
+	pfree(port->peer_cn);
+	port->peer_cn = NULL;
 
-	if (port->peer_dn)
-	{
-		pfree(port->peer_dn);
-		port->peer_dn = NULL;
-	}
+	pfree(port->peer_dn);
+	port->peer_dn = NULL;
 }
 
 ssize_t
diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index 0a6c30e4eb..6e401c1902 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -202,13 +202,12 @@ bms_make_singleton(int x)
 /*
  * bms_free - free a bitmapset
  *
- * Same as pfree except for allowing NULL input
+ * Same as pfree
  */
 void
 bms_free(Bitmapset *a)
 {
-	if (a)
-		pfree(a);
+	pfree(a);
 }
 
 
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index a7a6b26668..c22fa2ec50 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -322,10 +322,8 @@ tbm_free(TIDBitmap *tbm)
 {
 	if (tbm->pagetable)
 		pagetable_destroy(tbm->pagetable);
-	if (tbm->spages)
-		pfree(tbm->spages);
-	if (tbm->schunks)
-		pfree(tbm->schunks);
+	pfree(tbm->spages);
+	pfree(tbm->schunks);
 	pfree(tbm);
 }
 
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index cf9e0a74db..9e82aa2b4c 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3059,8 +3059,7 @@ extract_rollup_sets(List *groupingSets)
 	pfree(results);
 	pfree(chains);
 	for (i = 1; i <= num_sets; ++i)
-		if (adjacency[i])
-			pfree(adjacency[i]);
+		pfree(adjacency[i]);
 	pfree(adjacency);
 	pfree(adjacency_buf);
 	pfree(orig_sets);
diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l
index 882e081aae..2b3e6ec810 100644
--- a/src/backend/parser/scan.l
+++ b/src/backend/parser/scan.l
@@ -1433,6 +1433,5 @@ core_yyrealloc(void *ptr, yy_size_t bytes, core_yyscan_t yyscanner)
 void
 core_yyfree(void *ptr, core_yyscan_t yyscanner)
 {
-	if (ptr)
-		pfree(ptr);
+	pfree(ptr);
 }
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 70a9176c54..a9edc96027 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2511,12 +2511,9 @@ do_autovacuum(void)
 
 		/* be tidy */
 deleted:
-		if (tab->at_datname != NULL)
-			pfree(tab->at_datname);
-		if (tab->at_nspname != NULL)
-			pfree(tab->at_nspname);
-		if (tab->at_relname != NULL)
-			pfree(tab->at_relname);
+		pfree(tab->at_datname);
+		pfree(tab->at_nspname);
+		pfree(tab->at_relname);
 		pfree(tab);
 
 		/*
@@ -2702,12 +2699,9 @@ perform_work_item(AutoVacuumWorkItem *workitem)
 
 	/* be tidy */
 deleted2:
-	if (cur_datname)
-		pfree(cur_datname);
-	if (cur_nspname)
-		pfree(cur_nspname);
-	if (cur_relname)
-		pfree(cur_relname);
+	pfree(cur_datname);
+	pfree(cur_nspname);
+	pfree(cur_relname);
 }
 
 /*
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 5fc076fc14..f490e106a9 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -1308,8 +1308,7 @@ AbsorbSyncRequests(void)
 
 	END_CRIT_SECTION();
 
-	if (requests)
-		pfree(requests);
+	pfree(requests);
 }
 
 /*
diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index d6d02e3c63..5b3db40bee 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -1329,8 +1329,7 @@ logfile_rotate_dest(bool time_based_rotation, int size_rotation_for,
 		if (*logFile != NULL)
 			fclose(*logFile);
 		*logFile = NULL;
-		if (*last_file_name != NULL)
-			pfree(*last_file_name);
+		pfree(*last_file_name);
 		*last_file_name = NULL;
 		return true;
 	}
@@ -1386,8 +1385,7 @@ logfile_rotate_dest(bool time_based_rotation, int size_rotation_for,
 			rotation_disabled = true;
 		}
 
-		if (filename)
-			pfree(filename);
+		pfree(filename);
 		return false;
 	}
 
@@ -1397,8 +1395,7 @@ logfile_rotate_dest(bool time_based_rotation, int size_rotation_for,
 	*logFile = fh;
 
 	/* instead of pfree'ing filename, remember it for next time */
-	if (*last_file_name != NULL)
-		pfree(*last_file_name);
+	pfree(*last_file_name);
 	*last_file_name = filename;
 
 	return true;
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 1c21a1d14b..5628fff12e 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -437,23 +437,14 @@ ReorderBufferReturnTXN(ReorderBuffer *rb, ReorderBufferTXN *txn)
 
 	/* free data that's contained */
 
-	if (txn->gid != NULL)
-	{
-		pfree(txn->gid);
-		txn->gid = NULL;
-	}
+	pfree(txn->gid);
+	txn->gid = NULL;
 
-	if (txn->tuplecid_hash != NULL)
-	{
-		hash_destroy(txn->tuplecid_hash);
-		txn->tuplecid_hash = NULL;
-	}
+	hash_destroy(txn->tuplecid_hash);
+	txn->tuplecid_hash = NULL;
 
-	if (txn->invalidations)
-	{
-		pfree(txn->invalidations);
-		txn->invalidations = NULL;
-	}
+	pfree(txn->invalidations);
+	txn->invalidations = NULL;
 
 	/* Reset the toast hash */
 	ReorderBufferToastReset(rb, txn);
@@ -508,16 +499,13 @@ ReorderBufferReturnChange(ReorderBuffer *rb, ReorderBufferChange *change,
 			}
 			break;
 		case REORDER_BUFFER_CHANGE_MESSAGE:
-			if (change->data.msg.prefix != NULL)
-				pfree(change->data.msg.prefix);
+			pfree(change->data.msg.prefix);
 			change->data.msg.prefix = NULL;
-			if (change->data.msg.message != NULL)
-				pfree(change->data.msg.message);
+			pfree(change->data.msg.message);
 			change->data.msg.message = NULL;
 			break;
 		case REORDER_BUFFER_CHANGE_INVALIDATION:
-			if (change->data.inval.invalidations)
-				pfree(change->data.inval.invalidations);
+			pfree(change->data.inval.invalidations);
 			change->data.inval.invalidations = NULL;
 			break;
 		case REORDER_BUFFER_CHANGE_INTERNAL_SNAPSHOT:
@@ -4865,8 +4853,7 @@ ReorderBufferToastReset(ReorderBuffer *rb, ReorderBufferTXN *txn)
 	{
 		dlist_mutable_iter it;
 
-		if (ent->reconstructed != NULL)
-			pfree(ent->reconstructed);
+		pfree(ent->reconstructed);
 
 		dlist_foreach_modify(it, &ent->chunks)
 		{
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 1ff2c12240..08c98e6fbc 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1805,10 +1805,8 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
 	ReorderBufferSetRestartPoint(builder->reorder,
 								 builder->last_serialized_snapshot);
 	/* be tidy */
-	if (ondisk)
-		pfree(ondisk);
-	if (catchange_xip)
-		pfree(catchange_xip);
+	pfree(ondisk);
+	pfree(catchange_xip);
 }
 
 /*
@@ -1973,10 +1971,8 @@ SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn)
 	return true;
 
 snapshot_not_interesting:
-	if (ondisk.builder.committed.xip != NULL)
-		pfree(ondisk.builder.committed.xip);
-	if (ondisk.builder.catchange.xip != NULL)
-		pfree(ondisk.builder.catchange.xip);
+	pfree(ondisk.builder.committed.xip);
+	pfree(ondisk.builder.catchange.xip);
 	return false;
 }
 
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 5f8c541763..7468938960 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3464,9 +3464,7 @@ stream_write_change(char action, StringInfo s)
 static inline void
 cleanup_subxact_info()
 {
-	if (subxact_data.subxacts)
-		pfree(subxact_data.subxacts);
-
+	pfree(subxact_data.subxacts);
 	subxact_data.subxacts = NULL;
 	subxact_data.subxact_last = InvalidTransactionId;
 	subxact_data.nsubxacts = 0;
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 8604fd4bc2..5e8979f9dd 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -311,11 +311,8 @@ WalReceiverMain(void)
 	walrcv->ready_to_display = true;
 	SpinLockRelease(&walrcv->mutex);
 
-	if (tmp_conninfo)
-		pfree(tmp_conninfo);
-
-	if (sender_host)
-		pfree(sender_host);
+	pfree(tmp_conninfo);
+	pfree(sender_host);
 
 	first_stream = true;
 	for (;;)
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 990e081aae..417faa6440 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -595,9 +595,7 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 void
 FreeAccessStrategy(BufferAccessStrategy strategy)
 {
-	/* don't crash if called on a "default" strategy */
-	if (strategy != NULL)
-		pfree(strategy);
+	pfree(strategy);
 }
 
 /*
diff --git a/src/backend/storage/ipc/shm_mq.c b/src/backend/storage/ipc/shm_mq.c
index 8ca24de8d6..e77d8db710 100644
--- a/src/backend/storage/ipc/shm_mq.c
+++ b/src/backend/storage/ipc/shm_mq.c
@@ -860,8 +860,7 @@ shm_mq_detach(shm_mq_handle *mqh)
 							 PointerGetDatum(mqh->mqh_queue));
 
 	/* Release local memory associated with handle. */
-	if (mqh->mqh_buffer != NULL)
-		pfree(mqh->mqh_buffer);
+	pfree(mqh->mqh_buffer);
 	pfree(mqh);
 }
 
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 5f5803f681..12a375fac9 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -1374,8 +1374,7 @@ RemoveLocalLock(LOCALLOCK *locallock)
 			ResourceOwnerForgetLock(locallock->lockOwners[i].owner, locallock);
 	}
 	locallock->numLockOwners = 0;
-	if (locallock->lockOwners != NULL)
-		pfree(locallock->lockOwners);
+	pfree(locallock->lockOwners);
 	locallock->lockOwners = NULL;
 
 	if (locallock->holdsStrongLockCount)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 7bec4e4ff5..553d471c62 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2527,8 +2527,7 @@ bind_param_error_callback(void *arg)
 					   data->paramno + 1);
 	}
 
-	if (quotedval)
-		pfree(quotedval);
+	pfree(quotedval);
 }
 
 /*
diff --git a/src/backend/tsearch/dict_thesaurus.c b/src/backend/tsearch/dict_thesaurus.c
index b8c08bcf7b..ad838608ba 100644
--- a/src/backend/tsearch/dict_thesaurus.c
+++ b/src/backend/tsearch/dict_thesaurus.c
@@ -456,8 +456,7 @@ compileTheLexeme(DictThesaurus *d)
 		pfree(d->wrds[i].entries);
 	}
 
-	if (d->wrds)
-		pfree(d->wrds);
+	pfree(d->wrds);
 	d->wrds = newwrds;
 	d->nwrds = nnw;
 	d->ntwrds = tnm;
@@ -481,8 +480,7 @@ compileTheLexeme(DictThesaurus *d)
 				else
 					pfree(ptrwrds->entries);
 
-				if (ptrwrds->lexeme)
-					pfree(ptrwrds->lexeme);
+				pfree(ptrwrds->lexeme);
 			}
 			else
 			{
@@ -575,8 +573,7 @@ compileTheSubstitute(DictThesaurus *d)
 								inptr->lexeme, i + 1)));
 			}
 
-			if (inptr->lexeme)
-				pfree(inptr->lexeme);
+			pfree(inptr->lexeme);
 			inptr++;
 		}
 
diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index f07321b61d..372e3c0fe0 100644
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -1322,8 +1322,7 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
 
 		fields_read = parse_ooaffentry(recoded, type, sflag, find, repl, mask);
 
-		if (ptype)
-			pfree(ptype);
+		pfree(ptype);
 		ptype = lowerstr_ctx(Conf, type);
 
 		/* First try to parse AF parameter (alias compression) */
@@ -1427,8 +1426,7 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
 	}
 
 	tsearch_readline_end(&trst);
-	if (ptype)
-		pfree(ptype);
+	pfree(ptype);
 }
 
 /*
diff --git a/src/backend/tsearch/to_tsany.c b/src/backend/tsearch/to_tsany.c
index edeffacc2d..6a1f4f4b40 100644
--- a/src/backend/tsearch/to_tsany.c
+++ b/src/backend/tsearch/to_tsany.c
@@ -234,8 +234,7 @@ make_tsvector(ParsedText *prs)
 		ptr++;
 	}
 
-	if (prs->words)
-		pfree(prs->words);
+	pfree(prs->words);
 
 	return in;
 }
diff --git a/src/backend/tsearch/wparser_def.c b/src/backend/tsearch/wparser_def.c
index 826027844e..b6724ae7b3 100644
--- a/src/backend/tsearch/wparser_def.c
+++ b/src/backend/tsearch/wparser_def.c
@@ -378,10 +378,8 @@ TParserClose(TParser *prs)
 		prs->state = ptr;
 	}
 
-	if (prs->wstr)
-		pfree(prs->wstr);
-	if (prs->pgwstr)
-		pfree(prs->pgwstr);
+	pfree(prs->wstr);
+	pfree(prs->pgwstr);
 
 #ifdef WPARSER_TRACE
 	fprintf(stderr, "closing parser\n");
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 89060ef29a..37b2ce36ad 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -563,8 +563,7 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref,
 	if (!pgstat_entry_ref_hash_delete(pgStatEntryRefHash, key))
 		elog(ERROR, "entry ref vanished before deletion");
 
-	if (entry_ref)
-		pfree(entry_ref);
+	pfree(entry_ref);
 }
 
 bool
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 5318eda9cf..bc26bb8117 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -1281,10 +1281,8 @@ JsonbDeepContains(JsonbIterator **val, JsonbIterator **mContained)
 
 					contains = JsonbDeepContains(&nestval, &nestContained);
 
-					if (nestval)
-						pfree(nestval);
-					if (nestContained)
-						pfree(nestContained);
+					pfree(nestval);
+					pfree(nestContained);
 					if (contains)
 						break;
 				}
diff --git a/src/backend/utils/adt/jsonpath_scan.l b/src/backend/utils/adt/jsonpath_scan.l
index 4351f6ec98..2ed8bb0bc0 100644
--- a/src/backend/utils/adt/jsonpath_scan.l
+++ b/src/backend/utils/adt/jsonpath_scan.l
@@ -614,6 +614,5 @@ jsonpath_yyrealloc(void *ptr, yy_size_t bytes)
 void
 jsonpath_yyfree(void *ptr)
 {
-	if (ptr)
-		pfree(ptr);
+	pfree(ptr);
 }
diff --git a/src/backend/utils/adt/like_support.c b/src/backend/utils/adt/like_support.c
index 65a57fc3c4..a16c2f76a7 100644
--- a/src/backend/utils/adt/like_support.c
+++ b/src/backend/utils/adt/like_support.c
@@ -1687,8 +1687,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 											   workstr_const->constvalue)))
 			{
 				/* Successfully made a string larger than cmpstr */
-				if (cmptxt)
-					pfree(cmptxt);
+				pfree(cmptxt);
 				pfree(workstr);
 				return workstr_const;
 			}
@@ -1707,8 +1706,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 	}
 
 	/* Failed... */
-	if (cmptxt)
-		pfree(cmptxt);
+	pfree(cmptxt);
 	pfree(workstr);
 
 	return NULL;
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 920a63b008..2ffafcc1f5 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -477,10 +477,7 @@ static void dump_var(const char *str, NumericVar *var);
 #define digitbuf_alloc(ndigits)  \
 	((NumericDigit *) palloc((ndigits) * sizeof(NumericDigit)))
 #define digitbuf_free(buf)	\
-	do { \
-		 if ((buf) != NULL) \
-			 pfree(buf); \
-	} while (0)
+	pfree(buf)
 
 #define init_var(v)		memset(v, 0, sizeof(NumericVar))
 
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 1a047a97d7..b5f141d349 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -722,8 +722,7 @@ cache_single_string(char **dst, const char *src, int encoding)
 	/* Store the string in long-lived storage, replacing any previous value */
 	olddst = *dst;
 	*dst = MemoryContextStrdup(TopMemoryContext, ptr);
-	if (olddst)
-		pfree(olddst);
+	pfree(olddst);
 
 	/* Might as well clean up any palloc'd conversion result, too */
 	if (ptr != src)
diff --git a/src/backend/utils/adt/tsvector_op.c b/src/backend/utils/adt/tsvector_op.c
index 1786c18f89..41f4156373 100644
--- a/src/backend/utils/adt/tsvector_op.c
+++ b/src/backend/utils/adt/tsvector_op.c
@@ -1377,8 +1377,7 @@ checkcondition_str(void *checkval, QueryOperand *val, ExecPhraseData *data)
 						/* forget any previous positions */
 						npos = 0;
 						/* don't leak storage */
-						if (allpos)
-							pfree(allpos);
+						pfree(allpos);
 						break;
 					}
 
diff --git a/src/backend/utils/adt/tsvector_parser.c b/src/backend/utils/adt/tsvector_parser.c
index e2460d393a..4215549e3a 100644
--- a/src/backend/utils/adt/tsvector_parser.c
+++ b/src/backend/utils/adt/tsvector_parser.c
@@ -99,7 +99,7 @@ do { \
 		*pos_ptr = pos; \
 		*poslen = npos; \
 	} \
-	else if (pos != NULL) \
+	else \
 		pfree(pos); \
 	\
 	if (strval != NULL) \
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 8539cef024..13f343cec9 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2621,8 +2621,7 @@ varstr_abbrev_convert(Datum original, SortSupport ssup)
 		memcpy(pres, sss->buf2, Min(sizeof(Datum), bsize));
 
 #ifdef USE_ICU
-		if (uchar)
-			pfree(uchar);
+		pfree(uchar);
 #endif
 	}
 
@@ -5919,10 +5918,8 @@ text_format(PG_FUNCTION_ARGS)
 	}
 
 	/* Don't need deconstruct_array results anymore. */
-	if (elements != NULL)
-		pfree(elements);
-	if (nulls != NULL)
-		pfree(nulls);
+	pfree(elements);
+	pfree(nulls);
 
 	/* Generate results. */
 	result = cstring_to_text_with_len(str.data, str.len);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 60a85c4697..82da805250 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -1677,9 +1677,7 @@ xml_repalloc(void *ptr, size_t size)
 static void
 xml_pfree(void *ptr)
 {
-	/* At least some parts of libxml assume xmlFree(NULL) is allowed */
-	if (ptr)
-		pfree(ptr);
+	pfree(ptr);
 }
 
 
diff --git a/src/backend/utils/cache/attoptcache.c b/src/backend/utils/cache/attoptcache.c
index 9e252a0891..3e34baf168 100644
--- a/src/backend/utils/cache/attoptcache.c
+++ b/src/backend/utils/cache/attoptcache.c
@@ -59,8 +59,7 @@ InvalidateAttoptCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
 	hash_seq_init(&status, AttoptCacheHash);
 	while ((attopt = (AttoptCacheEntry *) hash_seq_search(&status)) != NULL)
 	{
-		if (attopt->opts)
-			pfree(attopt->opts);
+		pfree(attopt->opts);
 		if (hash_search(AttoptCacheHash,
 						(void *) &attopt->key,
 						HASH_REMOVE,
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index a16a63f495..75233060ff 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3305,14 +3305,11 @@ void
 free_attstatsslot(AttStatsSlot *sslot)
 {
 	/* The values[] array was separately palloc'd by deconstruct_array */
-	if (sslot->values)
-		pfree(sslot->values);
+	pfree(sslot->values);
 	/* The numbers[] array points into numbers_arr, do not pfree it */
 	/* Free the detoasted array objects, if any */
-	if (sslot->values_arr)
-		pfree(sslot->values_arr);
-	if (sslot->numbers_arr)
-		pfree(sslot->numbers_arr);
+	pfree(sslot->values_arr);
+	pfree(sslot->numbers_arr);
 }
 
 /*				---------- PG_NAMESPACE CACHE ----------				 */
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 00dc0f2403..1a5e746747 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2230,8 +2230,7 @@ RelationReloadIndexInfo(Relation relation)
 	RelationCloseSmgr(relation);
 
 	/* Must free any AM cached data upon relcache flush */
-	if (relation->rd_amcache)
-		pfree(relation->rd_amcache);
+	pfree(relation->rd_amcache);
 	relation->rd_amcache = NULL;
 
 	/*
@@ -2261,8 +2260,7 @@ RelationReloadIndexInfo(Relation relation)
 	relp = (Form_pg_class) GETSTRUCT(pg_class_tuple);
 	memcpy(relation->rd_rel, relp, CLASS_TUPLE_SIZE);
 	/* Reload reloptions in case they changed */
-	if (relation->rd_options)
-		pfree(relation->rd_options);
+	pfree(relation->rd_options);
 	RelationParseRelOptions(relation, pg_class_tuple);
 	/* done with pg_class tuple */
 	heap_freetuple(pg_class_tuple);
@@ -2417,8 +2415,7 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
 	 * Free all the subsidiary data structures of the relcache entry, then the
 	 * entry itself.
 	 */
-	if (relation->rd_rel)
-		pfree(relation->rd_rel);
+	pfree(relation->rd_rel);
 	/* can't use DecrTupleDescRefCount here */
 	Assert(relation->rd_att->tdrefcount > 0);
 	if (--relation->rd_att->tdrefcount == 0)
@@ -2444,16 +2441,11 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc)
 	bms_free(relation->rd_keyattr);
 	bms_free(relation->rd_pkattr);
 	bms_free(relation->rd_idattr);
-	if (relation->rd_pubdesc)
-		pfree(relation->rd_pubdesc);
-	if (relation->rd_options)
-		pfree(relation->rd_options);
-	if (relation->rd_indextuple)
-		pfree(relation->rd_indextuple);
-	if (relation->rd_amcache)
-		pfree(relation->rd_amcache);
-	if (relation->rd_fdwroutine)
-		pfree(relation->rd_fdwroutine);
+	pfree(relation->rd_pubdesc);
+	pfree(relation->rd_options);
+	pfree(relation->rd_indextuple);
+	pfree(relation->rd_amcache);
+	pfree(relation->rd_fdwroutine);
 	if (relation->rd_indexcxt)
 		MemoryContextDelete(relation->rd_indexcxt);
 	if (relation->rd_rulescxt)
@@ -2513,8 +2505,7 @@ RelationClearRelation(Relation relation, bool rebuild)
 	RelationCloseSmgr(relation);
 
 	/* Free AM cached data, if any */
-	if (relation->rd_amcache)
-		pfree(relation->rd_amcache);
+	pfree(relation->rd_amcache);
 	relation->rd_amcache = NULL;
 
 	/*
@@ -4202,8 +4193,7 @@ RelationCacheInitializePhase3(void)
 			memcpy((char *) relation->rd_rel, (char *) relp, CLASS_TUPLE_SIZE);
 
 			/* Update rd_options while we have the tuple */
-			if (relation->rd_options)
-				pfree(relation->rd_options);
+			pfree(relation->rd_options);
 			RelationParseRelOptions(relation, htup);
 
 			/*
@@ -5742,11 +5732,8 @@ RelationBuildPublicationDesc(Relation relation, PublicationDesc *pubdesc)
 			break;
 	}
 
-	if (relation->rd_pubdesc)
-	{
-		pfree(relation->rd_pubdesc);
-		relation->rd_pubdesc = NULL;
-	}
+	pfree(relation->rd_pubdesc);
+	relation->rd_pubdesc = NULL;
 
 	/* Now save copy of the descriptor in the relcache entry. */
 	oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
@@ -5829,8 +5816,7 @@ RelationGetIndexAttOptions(Relation relation, bool copy)
 
 			opts[i] = index_opclass_options(relation, i + 1, attoptions, false);
 
-			if (attoptions != (Datum) 0)
-				pfree(DatumGetPointer(attoptions));
+			pfree(DatumGetPointer(attoptions));
 		}
 	}
 
@@ -5843,10 +5829,7 @@ RelationGetIndexAttOptions(Relation relation, bool copy)
 		return opts;
 
 	for (i = 0; i < natts; i++)
-	{
-		if (opts[i])
-			pfree(opts[i]);
-	}
+		pfree(opts[i]);
 
 	pfree(opts);
 
diff --git a/src/backend/utils/cache/spccache.c b/src/backend/utils/cache/spccache.c
index 5609246c2f..db4e997a96 100644
--- a/src/backend/utils/cache/spccache.c
+++ b/src/backend/utils/cache/spccache.c
@@ -59,8 +59,7 @@ InvalidateTableSpaceCacheCallback(Datum arg, int cacheid, uint32 hashvalue)
 	hash_seq_init(&status, TableSpaceCacheHash);
 	while ((spc = (TableSpaceCacheEntry *) hash_seq_search(&status)) != NULL)
 	{
-		if (spc->opts)
-			pfree(spc->opts);
+		pfree(spc->opts);
 		if (hash_search(TableSpaceCacheHash,
 						(void *) &spc->oid,
 						HASH_REMOVE,
diff --git a/src/backend/utils/cache/ts_cache.c b/src/backend/utils/cache/ts_cache.c
index 24808dfbb1..1fe52a8359 100644
--- a/src/backend/utils/cache/ts_cache.c
+++ b/src/backend/utils/cache/ts_cache.c
@@ -449,8 +449,7 @@ lookup_ts_config_cache(Oid cfgId)
 			if (entry->map)
 			{
 				for (i = 0; i < entry->lenmap; i++)
-					if (entry->map[i].dictIds)
-						pfree(entry->map[i].dictIds);
+					pfree(entry->map[i].dictIds);
 				pfree(entry->map);
 			}
 		}
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 808f9ebd0d..20423bab72 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -2690,8 +2690,7 @@ load_enum_cache_data(TypeCacheEntry *tcache)
 	bms_free(bitmap);
 
 	/* And link the finished cache struct into the typcache */
-	if (tcache->enumData != NULL)
-		pfree(tcache->enumData);
+	pfree(tcache->enumData);
 	tcache->enumData = enumdata;
 }
 
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 95f32de4e2..572b6923e8 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -592,30 +592,18 @@ errfinish(const char *filename, int lineno, const char *funcname)
 	EmitErrorReport();
 
 	/* Now free up subsidiary data attached to stack entry, and release it */
-	if (edata->message)
-		pfree(edata->message);
-	if (edata->detail)
-		pfree(edata->detail);
-	if (edata->detail_log)
-		pfree(edata->detail_log);
-	if (edata->hint)
-		pfree(edata->hint);
-	if (edata->context)
-		pfree(edata->context);
-	if (edata->backtrace)
-		pfree(edata->backtrace);
-	if (edata->schema_name)
-		pfree(edata->schema_name);
-	if (edata->table_name)
-		pfree(edata->table_name);
-	if (edata->column_name)
-		pfree(edata->column_name);
-	if (edata->datatype_name)
-		pfree(edata->datatype_name);
-	if (edata->constraint_name)
-		pfree(edata->constraint_name);
-	if (edata->internalquery)
-		pfree(edata->internalquery);
+	pfree(edata->message);
+	pfree(edata->detail);
+	pfree(edata->detail_log);
+	pfree(edata->hint);
+	pfree(edata->context);
+	pfree(edata->backtrace);
+	pfree(edata->schema_name);
+	pfree(edata->table_name);
+	pfree(edata->column_name);
+	pfree(edata->datatype_name);
+	pfree(edata->constraint_name);
+	pfree(edata->internalquery);
 
 	errordata_stack_depth--;
 
@@ -845,8 +833,7 @@ errcode_for_socket_access(void)
 			enlargeStringInfo(&buf, needed); \
 		} \
 		/* Save the completed message into the stack item */ \
-		if (edata->targetfield) \
-			pfree(edata->targetfield); \
+		pfree(edata->targetfield); \
 		edata->targetfield = pstrdup(buf.data); \
 		pfree(buf.data); \
 	}
@@ -884,8 +871,7 @@ errcode_for_socket_access(void)
 			enlargeStringInfo(&buf, needed); \
 		} \
 		/* Save the completed message into the stack item */ \
-		if (edata->targetfield) \
-			pfree(edata->targetfield); \
+		pfree(edata->targetfield); \
 		edata->targetfield = pstrdup(buf.data); \
 		pfree(buf.data); \
 	}
@@ -1320,11 +1306,8 @@ internalerrquery(const char *query)
 	/* we don't bother incrementing recursion_depth */
 	CHECK_STACK_DEPTH();
 
-	if (edata->internalquery)
-	{
-		pfree(edata->internalquery);
-		edata->internalquery = NULL;
-	}
+	pfree(edata->internalquery);
+	edata->internalquery = NULL;
 
 	if (query)
 		edata->internalquery = MemoryContextStrdup(edata->assoc_context, query);
@@ -1610,30 +1593,18 @@ CopyErrorData(void)
 void
 FreeErrorData(ErrorData *edata)
 {
-	if (edata->message)
-		pfree(edata->message);
-	if (edata->detail)
-		pfree(edata->detail);
-	if (edata->detail_log)
-		pfree(edata->detail_log);
-	if (edata->hint)
-		pfree(edata->hint);
-	if (edata->context)
-		pfree(edata->context);
-	if (edata->backtrace)
-		pfree(edata->backtrace);
-	if (edata->schema_name)
-		pfree(edata->schema_name);
-	if (edata->table_name)
-		pfree(edata->table_name);
-	if (edata->column_name)
-		pfree(edata->column_name);
-	if (edata->datatype_name)
-		pfree(edata->datatype_name);
-	if (edata->constraint_name)
-		pfree(edata->constraint_name);
-	if (edata->internalquery)
-		pfree(edata->internalquery);
+	pfree(edata->message);
+	pfree(edata->detail);
+	pfree(edata->detail_log);
+	pfree(edata->hint);
+	pfree(edata->context);
+	pfree(edata->backtrace);
+	pfree(edata->schema_name);
+	pfree(edata->table_name);
+	pfree(edata->column_name);
+	pfree(edata->datatype_name);
+	pfree(edata->constraint_name);
+	pfree(edata->internalquery);
 	pfree(edata);
 }
 
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index bd973ba613..3a6e2b3028 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1667,8 +1667,7 @@ load_libraries(const char *libraries, const char *gucname, bool restricted)
 		load_file(filename, restricted);
 		ereport(DEBUG1,
 				(errmsg_internal("loaded library \"%s\"", filename)));
-		if (expanded)
-			pfree(expanded);
+		pfree(expanded);
 	}
 
 	list_free_deep(elemlist);
diff --git a/src/backend/utils/mb/stringinfo_mb.c b/src/backend/utils/mb/stringinfo_mb.c
index 4401dfc47a..25e872fa99 100644
--- a/src/backend/utils/mb/stringinfo_mb.c
+++ b/src/backend/utils/mb/stringinfo_mb.c
@@ -81,6 +81,5 @@ appendStringInfoStringQuoted(StringInfo str, const char *s, int maxlen)
 	else
 		appendStringInfo(str, "%s'", chunk_copy_start);
 
-	if (copy)
-		pfree(copy);
+	pfree(copy);
 }
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index ce5633844c..6d0e7b4dc5 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -478,8 +478,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel)
 			set_config_sourcefile(item->name, item->filename,
 								  item->sourceline);
 
-		if (pre_value)
-			pfree(pre_value);
+		pfree(pre_value);
 	}
 
 	/* Remember when we last successfully loaded the config file. */
@@ -883,10 +882,8 @@ ParseConfigFp(FILE *fp, const char *config_file, int depth, int elevel,
 
 parse_error:
 		/* release storage if we allocated any on this line */
-		if (opt_name)
-			pfree(opt_name);
-		if (opt_value)
-			pfree(opt_value);
+		pfree(opt_name);
+		pfree(opt_value);
 
 		/* report the error */
 		if (token == GUC_EOL || token == 0)
@@ -1119,14 +1116,10 @@ FreeConfigVariables(ConfigVariable *list)
 static void
 FreeConfigVariable(ConfigVariable *item)
 {
-	if (item->name)
-		pfree(item->name);
-	if (item->value)
-		pfree(item->value);
-	if (item->errmsg)
-		pfree(item->errmsg);
-	if (item->filename)
-		pfree(item->filename);
+	pfree(item->name);
+	pfree(item->value);
+	pfree(item->errmsg);
+	pfree(item->filename);
 	pfree(item);
 }
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 9fbbfb1be5..ada7bc6b90 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7513,8 +7513,7 @@ parse_and_validate_value(struct config_generic *record,
 									name, value),
 							 hintmsg ? errhint("%s", _(hintmsg)) : 0));
 
-					if (hintmsg)
-						pfree(hintmsg);
+					pfree(hintmsg);
 					return false;
 				}
 
diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index ceb4b0e3f7..1dbf41396b 100644
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -407,8 +407,7 @@ ResourceArrayGetAny(ResourceArray *resarr, Datum *value)
 static void
 ResourceArrayFree(ResourceArray *resarr)
 {
-	if (resarr->itemsarr)
-		pfree(resarr->itemsarr);
+	pfree(resarr->itemsarr);
 }
 
 
diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index c384f98e13..67cc6fd204 100644
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -733,8 +733,7 @@ ltsCreateTape(LogicalTapeSet *lts)
 void
 LogicalTapeClose(LogicalTape *lt)
 {
-	if (lt->buffer)
-		pfree(lt->buffer);
+	pfree(lt->buffer);
 	pfree(lt);
 }
 
@@ -901,8 +900,7 @@ LogicalTapeRewindForRead(LogicalTape *lt, size_t buffer_size)
 		Assert(lt->frozen);
 	}
 
-	if (lt->buffer)
-		pfree(lt->buffer);
+	pfree(lt->buffer);
 
 	/* the buffer is lazily allocated, but set the size here */
 	lt->buffer = NULL;
@@ -1018,8 +1016,7 @@ LogicalTapeFreeze(LogicalTape *lt, TapeShare *share)
 	 */
 	if (!lt->buffer || lt->buffer_size != BLCKSZ)
 	{
-		if (lt->buffer)
-			pfree(lt->buffer);
+		pfree(lt->buffer);
 		lt->buffer = palloc(BLCKSZ);
 		lt->buffer_size = BLCKSZ;
 	}
diff --git a/src/backend/utils/sort/sharedtuplestore.c b/src/backend/utils/sort/sharedtuplestore.c
index 996cef07d4..daba9fdf95 100644
--- a/src/backend/utils/sort/sharedtuplestore.c
+++ b/src/backend/utils/sort/sharedtuplestore.c
@@ -444,8 +444,7 @@ sts_read_tuple(SharedTuplestoreAccessor *accessor, void *meta_data)
 	{
 		size_t		new_read_buffer_size;
 
-		if (accessor->read_buffer != NULL)
-			pfree(accessor->read_buffer);
+		pfree(accessor->read_buffer);
 		new_read_buffer_size = Max(size, accessor->read_buffer_size * 2);
 		accessor->read_buffer =
 			MemoryContextAlloc(accessor->context, new_read_buffer_size);
diff --git a/src/bin/pg_verifybackup/parse_manifest.c b/src/bin/pg_verifybackup/parse_manifest.c
index 6364b01282..d682babba3 100644
--- a/src/bin/pg_verifybackup/parse_manifest.c
+++ b/src/bin/pg_verifybackup/parse_manifest.c
@@ -535,21 +535,12 @@ json_manifest_finalize_file(JsonManifestParseState *parse)
 						checksum_type, checksum_length, checksum_payload);
 
 	/* Free memory we no longer need. */
-	if (parse->size != NULL)
-	{
-		pfree(parse->size);
-		parse->size = NULL;
-	}
-	if (parse->algorithm != NULL)
-	{
-		pfree(parse->algorithm);
-		parse->algorithm = NULL;
-	}
-	if (parse->checksum != NULL)
-	{
-		pfree(parse->checksum);
-		parse->checksum = NULL;
-	}
+	pfree(parse->size);
+	parse->size = NULL;
+	pfree(parse->algorithm);
+	parse->algorithm = NULL;
+	pfree(parse->checksum);
+	parse->checksum = NULL;
 }
 
 /*
@@ -591,21 +582,12 @@ json_manifest_finalize_wal_range(JsonManifestParseState *parse)
 	context->perwalrange_cb(context, tli, start_lsn, end_lsn);
 
 	/* Free memory we no longer need. */
-	if (parse->timeline != NULL)
-	{
-		pfree(parse->timeline);
-		parse->timeline = NULL;
-	}
-	if (parse->start_lsn != NULL)
-	{
-		pfree(parse->start_lsn);
-		parse->start_lsn = NULL;
-	}
-	if (parse->end_lsn != NULL)
-	{
-		pfree(parse->end_lsn);
-		parse->end_lsn = NULL;
-	}
+	pfree(parse->timeline);
+	parse->timeline = NULL;
+	pfree(parse->start_lsn);
+	parse->start_lsn = NULL;
+	pfree(parse->end_lsn);
+	parse->end_lsn = NULL;
 }
 
 /*
diff --git a/src/common/compression.c b/src/common/compression.c
index da3c291c0f..b63d42844f 100644
--- a/src/common/compression.c
+++ b/src/common/compression.c
@@ -188,8 +188,7 @@ parse_compress_specification(pg_compress_algorithm algorithm, char *specificatio
 
 		/* Release memory, just to be tidy. */
 		pfree(keyword);
-		if (value != NULL)
-			pfree(value);
+		pfree(value);
 
 		/*
 		 * If we got an error or have reached the end of the string, stop.
diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h
index 9339f29303..4780b2a0a9 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -442,8 +442,7 @@ walrcv_clear_result(WalRcvExecResult *walres)
 	if (!walres)
 		return;
 
-	if (walres->err)
-		pfree(walres->err);
+	pfree(walres->err);
 
 	if (walres->tuplestore)
 		tuplestore_end(walres->tuplestore);
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index af354a68cc..f6fc4303fd 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -4064,8 +4064,7 @@ plperl_util_elog(int level, SV *msg)
 		edata = CopyErrorData();
 		FlushErrorState();
 
-		if (cmsg)
-			pfree(cmsg);
+		pfree(cmsg);
 
 		/* Punt the error to Perl */
 		croak_cstr(edata->message);
diff --git a/src/pl/plpython/plpy_elog.c b/src/pl/plpython/plpy_elog.c
index 7c627eacfb..5058c6a205 100644
--- a/src/pl/plpython/plpy_elog.c
+++ b/src/pl/plpython/plpy_elog.c
@@ -141,10 +141,8 @@ PLy_elog_impl(int elevel, const char *fmt,...)
 	{
 		if (fmt)
 			pfree(emsg.data);
-		if (xmsg)
-			pfree(xmsg);
-		if (tbmsg)
-			pfree(tbmsg);
+		pfree(xmsg);
+		pfree(tbmsg);
 		Py_XDECREF(exc);
 		Py_XDECREF(val);
 	}
diff --git a/src/pl/plpython/plpy_exec.c b/src/pl/plpython/plpy_exec.c
index 150b3a5977..1a7c4b6a72 100644
--- a/src/pl/plpython/plpy_exec.c
+++ b/src/pl/plpython/plpy_exec.c
@@ -981,12 +981,9 @@ PLy_modify_tuple(PLyProcedure *proc, PyObject *pltd, TriggerData *tdata,
 		Py_XDECREF(plkeys);
 		Py_XDECREF(plval);
 
-		if (modvalues)
-			pfree(modvalues);
-		if (modnulls)
-			pfree(modnulls);
-		if (modrepls)
-			pfree(modrepls);
+		pfree(modvalues);
+		pfree(modnulls);
+		pfree(modrepls);
 
 		PG_RE_THROW();
 	}
diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c
index fa08f0dbfb..91ab319700 100644
--- a/src/pl/plpython/plpy_plpymodule.c
+++ b/src/pl/plpython/plpy_plpymodule.c
@@ -452,8 +452,7 @@ PLy_output(volatile int level, PyObject *self, PyObject *args, PyObject *kw)
 					return NULL;
 				}
 
-				if (message)
-					pfree(message);
+				pfree(message);
 				message = object_to_string(value);
 			}
 			else if (strcmp(keyword, "detail") == 0)
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 7018c9d404..9f9f5eec84 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -175,8 +175,7 @@ PLy_input_setup_tuple(PLyDatumToOb *arg, TupleDesc desc, PLyProcedure *proc)
 	/* (Re)allocate atts array as needed */
 	if (arg->u.tuple.natts != desc->natts)
 	{
-		if (arg->u.tuple.atts)
-			pfree(arg->u.tuple.atts);
+		pfree(arg->u.tuple.atts);
 		arg->u.tuple.natts = desc->natts;
 		arg->u.tuple.atts = (PLyDatumToOb *)
 			MemoryContextAllocZero(arg->mcxt,
@@ -225,8 +224,7 @@ PLy_output_setup_tuple(PLyObToDatum *arg, TupleDesc desc, PLyProcedure *proc)
 	/* (Re)allocate atts array as needed */
 	if (arg->u.tuple.natts != desc->natts)
 	{
-		if (arg->u.tuple.atts)
-			pfree(arg->u.tuple.atts);
+		pfree(arg->u.tuple.atts);
 		arg->u.tuple.natts = desc->natts;
 		arg->u.tuple.atts = (PLyObToDatum *)
 			MemoryContextAllocZero(arg->mcxt,
diff --git a/src/test/modules/test_oat_hooks/test_oat_hooks.c b/src/test/modules/test_oat_hooks/test_oat_hooks.c
index 4b4e259cd2..65f21b1209 100644
--- a/src/test/modules/test_oat_hooks/test_oat_hooks.c
+++ b/src/test/modules/test_oat_hooks/test_oat_hooks.c
@@ -249,10 +249,8 @@ emit_audit_message(const char *type, const char *hook, char *action, char *objNa
 					 errmsg("in %s: %s %s %s", hook, who, type, action)));
 	}
 
-	if (action)
-		pfree(action);
-	if (objName)
-		pfree(objName);
+	pfree(action);
+	pfree(objName);
 }
 
 static void
-- 
2.37.1

0004-Remove-unneeded-null-pointer-checks-before-PQfreemem.patchtext/plain; charset=UTF-8; name=0004-Remove-unneeded-null-pointer-checks-before-PQfreemem.patchDownload
From f8332e5d2797b1e7d4081a746412a49f8595d38f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Aug 2022 16:51:52 +0200
Subject: [PATCH 4/5] Remove unneeded null pointer checks before PQfreemem()

---
 contrib/vacuumlo/vacuumlo.c                              | 9 +++------
 .../replication/libpqwalreceiver/libpqwalreceiver.c      | 6 ++----
 src/bin/pg_basebackup/receivelog.c                       | 9 +++------
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index b7c8f2c805..264b879bd3 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -231,12 +231,9 @@ vacuumlo(const char *database, const struct _param *param)
 			pg_log_error("%s", PQerrorMessage(conn));
 			PQclear(res);
 			PQfinish(conn);
-			if (schema != NULL)
-				PQfreemem(schema);
-			if (table != NULL)
-				PQfreemem(table);
-			if (field != NULL)
-				PQfreemem(field);
+			PQfreemem(schema);
+			PQfreemem(table);
+			PQfreemem(field);
 			return -1;
 		}
 
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 2865024524..7f697b0f29 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -741,8 +741,7 @@ static void
 libpqrcv_disconnect(WalReceiverConn *conn)
 {
 	PQfinish(conn->streamConn);
-	if (conn->recvBuf != NULL)
-		PQfreemem(conn->recvBuf);
+	PQfreemem(conn->recvBuf);
 	pfree(conn);
 }
 
@@ -768,8 +767,7 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
 {
 	int			rawlen;
 
-	if (conn->recvBuf != NULL)
-		PQfreemem(conn->recvBuf);
+	PQfreemem(conn->recvBuf);
 	conn->recvBuf = NULL;
 
 	/* Try to receive a CopyData message */
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 976d1e73b1..5f6fd3201f 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -858,8 +858,7 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
 	}
 
 error:
-	if (copybuf != NULL)
-		PQfreemem(copybuf);
+	PQfreemem(copybuf);
 	return NULL;
 }
 
@@ -940,8 +939,7 @@ CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
 	char	   *copybuf = NULL;
 	int			rawlen;
 
-	if (*buffer != NULL)
-		PQfreemem(*buffer);
+	PQfreemem(*buffer);
 	*buffer = NULL;
 
 	/* Try to receive a CopyData message */
@@ -1204,8 +1202,7 @@ HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
 		}
 		still_sending = false;
 	}
-	if (copybuf != NULL)
-		PQfreemem(copybuf);
+	PQfreemem(copybuf);
 	*stoppos = blockpos;
 	return res;
 }
-- 
2.37.1

0005-Remove-unneeded-null-pointer-check-before-FreeDir.patchtext/plain; charset=UTF-8; name=0005-Remove-unneeded-null-pointer-check-before-FreeDir.patchDownload
From 0584fb2e6b09798ca025107ef49d5f28977cf31f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 22 Aug 2022 16:51:52 +0200
Subject: [PATCH 5/5] Remove unneeded null pointer check before FreeDir()

---
 src/backend/utils/misc/guc-file.l | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 6d0e7b4dc5..acc34858c5 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -1086,8 +1086,7 @@ ParseConfigDirectory(const char *includedir,
 	status = true;
 
 cleanup:
-	if (d)
-		FreeDir(d);
+	FreeDir(d);
 	pfree(directory);
 	return status;
 }
-- 
2.37.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Change pfree to accept NULL argument

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Per discussion in [0], here is a patch set that allows pfree() to accept
a NULL argument, like free() does.

So the question is, is this actually a good thing to do?

If we were starting in a green field, I'd be fine with defining
pfree(NULL) as okay. But we're not, so there are a couple of big
objections:

* Code developed to this standard will be unsafe to back-patch

* The sheer number of places touched will create back-patching
hazards.

I'm not very convinced that the benefits of making pfree() more
like free() are worth those costs.

We could ameliorate the first objection if we wanted to back-patch
0002, I guess.

(FWIW, no objection to your 0001. 0004 and 0005 seem okay too;
they don't touch enough places to create much back-patching risk.)

regards, tom lane

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: Change pfree to accept NULL argument

Hi,

On 2022-08-22 14:30:22 -0400, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Per discussion in [0], here is a patch set that allows pfree() to accept
a NULL argument, like free() does.

So the question is, is this actually a good thing to do?

If we were starting in a green field, I'd be fine with defining
pfree(NULL) as okay. But we're not, so there are a couple of big
objections:

* Code developed to this standard will be unsafe to back-patch

* The sheer number of places touched will create back-patching
hazards.

I'm not very convinced that the benefits of making pfree() more
like free() are worth those costs.

It's probably also not entirely cost free due to the added branches in place
we are certain that the pointer is non-null. That could partially be
ameliorated by moving the NULL pointer check into the callers.

If we don't want to go this route it might be worth adding a
pg_attribute_nonnull() or such to pfree().

(FWIW, no objection to your 0001. 0004 and 0005 seem okay too;
they don't touch enough places to create much back-patching risk.)

I like 0001, not sure I find 0004, 0005 an improvement.

Semi-related note: I've sometimes wished for a pfreep(void **p) that'd do
something like

if (*p)
{
pfree(*p);
*p = NULL;
}

so there's no dangling pointers after the pfree(), which often enoughis
important (e.g. because the code could be reached again if there's an error)
and is also helpful when debugging. The explicit form does bulk up code
sufficiently to be annoying.

Greetings,

Andres Freund

#4Ranier Vilela
ranier.vf@gmail.com
In reply to: Andres Freund (#3)
1 attachment(s)
re: Change pfree to accept NULL argument

Per discussion in [0], here is a patch set that allows pfree() to accept
a NULL argument, like free() does.

Also, a patch that removes the now-unnecessary null pointer checks
before calling pfree(). And a few patches that do the same for some
other functions that I found around. (The one with FreeDir() is perhaps
a bit arguable, since FreeDir() wraps closedir() which does *not* accept
NULL arguments. Also, neither FreeFile() nor the underlying fclose()
accept NULL.)

Hi Peter,

+1

However, after a quick review, I noticed some cases of PQ freemen missing.
I took the liberty of making a v1, attached.

regards,

Ranier Vilela

Attachments:

v1-0004-Remove-unneeded-null-pointer-checks-before-PQfreemem.patchapplication/octet-stream; name=v1-0004-Remove-unneeded-null-pointer-checks-before-PQfreemem.patchDownload
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
index b7c8f2c805..264b879bd3 100644
--- a/contrib/vacuumlo/vacuumlo.c
+++ b/contrib/vacuumlo/vacuumlo.c
@@ -231,12 +231,9 @@ vacuumlo(const char *database, const struct _param *param)
 			pg_log_error("%s", PQerrorMessage(conn));
 			PQclear(res);
 			PQfinish(conn);
-			if (schema != NULL)
-				PQfreemem(schema);
-			if (table != NULL)
-				PQfreemem(table);
-			if (field != NULL)
-				PQfreemem(field);
+			PQfreemem(schema);
+			PQfreemem(table);
+			PQfreemem(field);
 			return -1;
 		}
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 2865024524..7f697b0f29 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -741,8 +741,7 @@ static void
 libpqrcv_disconnect(WalReceiverConn *conn)
 {
 	PQfinish(conn->streamConn);
-	if (conn->recvBuf != NULL)
-		PQfreemem(conn->recvBuf);
+	PQfreemem(conn->recvBuf);
 	pfree(conn);
 }
 
@@ -768,8 +767,7 @@ libpqrcv_receive(WalReceiverConn *conn, char **buffer,
 {
 	int			rawlen;
 
-	if (conn->recvBuf != NULL)
-		PQfreemem(conn->recvBuf);
+	PQfreemem(conn->recvBuf);
 	conn->recvBuf = NULL;
 
 	/* Try to receive a CopyData message */
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index 2a4c8b130a..df71ff3931 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -277,11 +277,8 @@ StreamLogicalLog(void)
 		int			hdr_len;
 		XLogRecPtr	cur_record_lsn = InvalidXLogRecPtr;
 
-		if (copybuf != NULL)
-		{
-			PQfreemem(copybuf);
-			copybuf = NULL;
-		}
+		PQfreemem(copybuf);
+		copybuf = NULL;
 
 		/*
 		 * Potentially send a status message to the primary.
@@ -595,11 +592,8 @@ StreamLogicalLog(void)
 		{
 			int			r;
 
-			if (copybuf != NULL)
-			{
-				PQfreemem(copybuf);
-				copybuf = NULL;
-			}
+			PQfreemem(copybuf);
+			copybuf = NULL;
 			r = PQgetCopyData(conn, &copybuf, 0);
 			if (r == -1)
 				break;
@@ -634,11 +628,8 @@ StreamLogicalLog(void)
 	}
 	outfd = -1;
 error:
-	if (copybuf != NULL)
-	{
-		PQfreemem(copybuf);
-		copybuf = NULL;
-	}
+	PQfreemem(copybuf);
+	copybuf = NULL;
 	destroyPQExpBuffer(query);
 	PQfinish(conn);
 	conn = NULL;
diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 976d1e73b1..5f6fd3201f 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -858,8 +858,7 @@ HandleCopyStream(PGconn *conn, StreamCtl *stream,
 	}
 
 error:
-	if (copybuf != NULL)
-		PQfreemem(copybuf);
+	PQfreemem(copybuf);
 	return NULL;
 }
 
@@ -940,8 +939,7 @@ CopyStreamReceive(PGconn *conn, long timeout, pgsocket stop_socket,
 	char	   *copybuf = NULL;
 	int			rawlen;
 
-	if (*buffer != NULL)
-		PQfreemem(*buffer);
+	PQfreemem(*buffer);
 	*buffer = NULL;
 
 	/* Try to receive a CopyData message */
@@ -1204,8 +1202,7 @@ HandleEndOfCopyStream(PGconn *conn, StreamCtl *stream, char *copybuf,
 		}
 		still_sending = false;
 	}
-	if (copybuf != NULL)
-		PQfreemem(copybuf);
+	PQfreemem(copybuf);
 	*stoppos = blockpos;
 	return res;
 }
#5David Rowley
dgrowleyml@gmail.com
In reply to: Tom Lane (#2)
Re: Change pfree to accept NULL argument

On Tue, 23 Aug 2022 at 06:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

Per discussion in [0], here is a patch set that allows pfree() to accept
a NULL argument, like free() does.

So the question is, is this actually a good thing to do?

I think making pfree() accept NULL is a bad idea. The vast majority
of cases the pointer will never be NULL, so we're effectively just
burdening those with the additional overhead of checking for NULL.

We know from [1]/messages/by-id/CAApHDvr6qFw3jLBL9d4zUpo3A2Cb6hoZsUnWD0vF1OGsd67v=w@mail.gmail.com that adding branching in the memory management code
can be costly.

I'm measuring about a 2.6% slowdown from the 0002 patch using a
function that I wrote [2]/messages/by-id/attachment/136801/pg_allocate_memory_test.patch.txt to hammer palloc/pfree.

master
postgres=# select pg_allocate_memory_test(64, 1024*1024,
10::bigint*1024*1024*1024,'aset');
Time: 2007.527 ms (00:02.008)
Time: 1991.574 ms (00:01.992)
Time: 2008.945 ms (00:02.009)
Time: 2011.410 ms (00:02.011)
Time: 2019.317 ms (00:02.019)
Time: 2060.832 ms (00:02.061)
Time: 2003.066 ms (00:02.003)
Time: 2025.039 ms (00:02.025)
Time: 2039.744 ms (00:02.040)
Time: 2090.384 ms (00:02.090)

master + pfree modifed to check for NULLs
postgres=# select pg_allocate_memory_test(64, 1024*1024,
10::bigint*1024*1024*1024,'aset');
Time: 2057.625 ms (00:02.058)
Time: 2074.699 ms (00:02.075)
Time: 2075.629 ms (00:02.076)
Time: 2104.581 ms (00:02.105)
Time: 2072.620 ms (00:02.073)
Time: 2066.916 ms (00:02.067)
Time: 2071.962 ms (00:02.072)
Time: 2097.520 ms (00:02.098)
Time: 2087.421 ms (00:02.087)
Time: 2078.695 ms (00:02.079)

(~2.62% slowdown)

If the aim here is to remove a bunch of ugly if (ptr) pfree(ptr);
code, then why don't we just have a[n inline] function or a macro for
that and only use it when we need to?

David

[1]: /messages/by-id/CAApHDvr6qFw3jLBL9d4zUpo3A2Cb6hoZsUnWD0vF1OGsd67v=w@mail.gmail.com
[2]: /messages/by-id/attachment/136801/pg_allocate_memory_test.patch.txt

#6David Rowley
dgrowleyml@gmail.com
In reply to: David Rowley (#5)
Re: Change pfree to accept NULL argument

On Tue, 23 Aug 2022 at 13:17, David Rowley <dgrowleyml@gmail.com> wrote:

I think making pfree() accept NULL is a bad idea.

One counter argument to that is for cases like list_free_deep().
Right now if I'm not mistaken there's a bug (which I just noticed) in
list_free_private() that would trigger if you have a List of Lists and
one of the inner Lists is NIL. The code in list_free_private() just
seems to go off and pfree() whatever is stored in the element, which I
think would crash if it found a NIL List. If pfree() was to handle
NULLs at least that wouldn't have been a crash, but in reality, we
should probably fix that with recursion if we detect the element IsA
List type. If we don't use recursion, then the "free" does not seem
very "deep". (Or maybe it's too late to make it go deeper as it might
break existing code.)

David

#7David Rowley
dgrowleyml@gmail.com
In reply to: David Rowley (#6)
Re: Change pfree to accept NULL argument

On Wed, 24 Aug 2022 at 23:07, David Rowley <dgrowleyml@gmail.com> wrote:

One counter argument to that is for cases like list_free_deep().
Right now if I'm not mistaken there's a bug (which I just noticed) in
list_free_private() that would trigger if you have a List of Lists and
one of the inner Lists is NIL. The code in list_free_private() just
seems to go off and pfree() whatever is stored in the element, which I
think would crash if it found a NIL List. If pfree() was to handle
NULLs at least that wouldn't have been a crash, but in reality, we
should probably fix that with recursion if we detect the element IsA
List type. If we don't use recursion, then the "free" does not seem
very "deep". (Or maybe it's too late to make it go deeper as it might
break existing code.)

Hmm, that was a false alarm. It seems list_free_deep() can't really
handle freeing sublists as the list elements might be non-Node types,
which of course have no node tag, so we can't check for sub-Lists.

David

#8Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Tom Lane (#2)
Re: Change pfree to accept NULL argument

On 22.08.22 20:30, Tom Lane wrote:

I'm not very convinced that the benefits of making pfree() more
like free() are worth those costs.

We could ameliorate the first objection if we wanted to back-patch
0002, I guess.

(FWIW, no objection to your 0001. 0004 and 0005 seem okay too;
they don't touch enough places to create much back-patching risk.)

To conclude this, I have committed those secondary patches and updated
the utils/mmgr/README with some information from this discussion.