v2-0002-Remove-redundant-null-pointer-checks-before-pg_fr.patch

text/plain

Filename: v2-0002-Remove-redundant-null-pointer-checks-before-pg_fr.patch
Type: text/plain
Part: 1
Message: Re: libpq: Remove redundant null pointer checks before free()

Patch

Format: format-patch
Series: patch v2-0002
Subject: Remove redundant null pointer checks before pg_free()
File+
src/bin/pg_basebackup/walmethods.c 1 2
src/bin/pgbench/pgbench.c 3 6
src/bin/pg_upgrade/parallel.c 6 12
src/bin/psql/command.c 1 2
src/bin/psql/variables.c 2 3
From 6fc396a32347edf4fec7f772300ff13360641700 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 17 Jun 2022 11:51:38 +0200
Subject: [PATCH v2 2/3] Remove redundant null pointer checks before pg_free()

This deserves shaming in a separate commit because the whole point of
pg_free() was to do that very check before calling free().

Logically, pg_free() should be removed, but I'm keeping it here to
keep the API consistent.
---
 src/bin/pg_basebackup/walmethods.c |  3 +--
 src/bin/pg_upgrade/parallel.c      | 18 ++++++------------
 src/bin/pgbench/pgbench.c          |  9 +++------
 src/bin/psql/command.c             |  3 +--
 src/bin/psql/variables.c           |  5 ++---
 5 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index cc292718da..ef4c11277a 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -500,8 +500,7 @@ dir_close(Walfile f, WalCloseMethod method)
 
 	pg_free(df->pathname);
 	pg_free(df->fullpath);
-	if (df->temp_suffix)
-		pg_free(df->temp_suffix);
+	pg_free(df->temp_suffix);
 	pg_free(df);
 
 	return r;
diff --git a/src/bin/pg_upgrade/parallel.c b/src/bin/pg_upgrade/parallel.c
index ca40df7b4c..5a3df84f01 100644
--- a/src/bin/pg_upgrade/parallel.c
+++ b/src/bin/pg_upgrade/parallel.c
@@ -130,14 +130,11 @@ parallel_exec_prog(const char *log_file, const char *opt_log_file,
 		new_arg = exec_thread_args[parallel_jobs - 1];
 
 		/* Can only pass one pointer into the function, so use a struct */
-		if (new_arg->log_file)
-			pg_free(new_arg->log_file);
+		pg_free(new_arg->log_file);
 		new_arg->log_file = pg_strdup(log_file);
-		if (new_arg->opt_log_file)
-			pg_free(new_arg->opt_log_file);
+		pg_free(new_arg->opt_log_file);
 		new_arg->opt_log_file = opt_log_file ? pg_strdup(opt_log_file) : NULL;
-		if (new_arg->cmd)
-			pg_free(new_arg->cmd);
+		pg_free(new_arg->cmd);
 		new_arg->cmd = pg_strdup(cmd);
 
 		child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_exec_prog,
@@ -243,14 +240,11 @@ parallel_transfer_all_new_dbs(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
 		/* Can only pass one pointer into the function, so use a struct */
 		new_arg->old_db_arr = old_db_arr;
 		new_arg->new_db_arr = new_db_arr;
-		if (new_arg->old_pgdata)
-			pg_free(new_arg->old_pgdata);
+		pg_free(new_arg->old_pgdata);
 		new_arg->old_pgdata = pg_strdup(old_pgdata);
-		if (new_arg->new_pgdata)
-			pg_free(new_arg->new_pgdata);
+		pg_free(new_arg->new_pgdata);
 		new_arg->new_pgdata = pg_strdup(new_pgdata);
-		if (new_arg->old_tablespace)
-			pg_free(new_arg->old_tablespace);
+		pg_free(new_arg->old_tablespace);
 		new_arg->old_tablespace = old_tablespace ? pg_strdup(old_tablespace) : NULL;
 
 		child = (HANDLE) _beginthreadex(NULL, 0, (void *) win32_transfer_all_new_dbs,
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 36b5c8fff7..bcaea8f5ea 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5473,12 +5473,10 @@ static void
 free_command(Command *command)
 {
 	termPQExpBuffer(&command->lines);
-	if (command->first_line)
-		pg_free(command->first_line);
+	pg_free(command->first_line);
 	for (int i = 0; i < command->argc; i++)
 		pg_free(command->argv[i]);
-	if (command->varprefix)
-		pg_free(command->varprefix);
+	pg_free(command->varprefix);
 
 	/*
 	 * It should also free expr recursively, but this is currently not needed
@@ -6635,8 +6633,7 @@ main(int argc, char **argv)
 				is_init_mode = true;
 				break;
 			case 'I':
-				if (initialize_steps)
-					pg_free(initialize_steps);
+				pg_free(initialize_steps);
 				initialize_steps = pg_strdup(optarg);
 				checkInitSteps(initialize_steps);
 				initialization_option_set = true;
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4495f7a907..f3c5196c90 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3475,8 +3475,7 @@ do_connect(enum trivalue reuse_previous_specification,
 	}							/* end retry loop */
 
 	/* Release locally allocated data, whether we succeeded or not */
-	if (password)
-		pg_free(password);
+	pg_free(password);
 	if (cinfo)
 		PQconninfoFree(cinfo);
 
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index 47c58d2be9..5cc4bc0cb7 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -255,8 +255,7 @@ SetVariable(VariableSpace space, const char *name, const char *value)
 
 			if (confirmed)
 			{
-				if (current->value)
-					pg_free(current->value);
+				pg_free(current->value);
 				current->value = new_value;
 
 				/*
@@ -272,7 +271,7 @@ SetVariable(VariableSpace space, const char *name, const char *value)
 					free(current);
 				}
 			}
-			else if (new_value)
+			else
 				pg_free(new_value); /* current->value is left unchanged */
 
 			return confirmed;
-- 
2.36.1