Remove redundant null pointer checks before pg_free()

petere · petere · commit 098c703d308f · 2022-07-03T09:05:34.000+02:00
These are especially useless because the whole point of pg_free() was to do that very check before calling free(). pg_free() could be removed altogether, but I'm keeping it here to keep the API consistent. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com
diff --git 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
@@ -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
@@ -5475,12 +5475,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
@@ -6637,8 +6635,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
@@ -3492,8 +3492,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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -255,8 +255,7 @@ SetVariable(VariableSpace space, const char name, const char value)`
`255`	`255`
`256`	`256`	`if (confirmed)`
`257`	`257`	`{`
`258`		`- if (current->value)`
`259`		`- pg_free(current->value);`
	`258`	`+ pg_free(current->value);`
`260`	`259`	`current->value = new_value;`
`261`	`260`
`262`	`261`	`/*`
`@@ -272,7 +271,7 @@ SetVariable(VariableSpace space, const char name, const char value)`
`272`	`271`	`free(current);`
`273`	`272`	`}`
`274`	`273`	`}`
`275`		`- else if (new_value)`
	`274`	`+ else`
`276`	`275`	`pg_free(new_value); /* current->value is left unchanged */`
`277`	`276`
`278`	`277`	`return confirmed;`