Always pfree strings returned by GetDatabasePath

alvherre · alvherre · commit 0bd56172b287 · 2022-04-25T10:32:13.000+02:00
Several places didn't do it, and in many cases it didn't matter because it would be a small allocation in a short-lived context; but other places may accumulate a few (for example, in CreateDatabaseUsingFileCopy, one per tablespace). In most databases this is highly unlikely to be very serious either, but it seems better to make the code consistent in case there's future copy-and-paste. The only case of actual concern seems to be the aforementioned routine, which is new with commit 9c08aea, so there's no need to backpatch. As pointed out by Coverity.
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
@@ -217,6 +217,8 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid,
 		UnlockRelationId(&dstrelid, AccessShareLock);
 	}
 
+	pfree(srcpath);
+	pfree(dstpath);
 	list_free_deep(rnodelist);
 }
 
@@ -628,6 +630,8 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,
 			(void) XLogInsert(RM_DBASE_ID,
 							  XLOG_DBASE_CREATE_FILE_COPY | XLR_SPECIAL_REL_UPDATE);
 		}
+		pfree(srcpath);
+		pfree(dstpath);
 	}
 	table_endscan(scan);
 	table_close(rel, AccessShareLock);
@@ -2128,6 +2132,9 @@ movedb(const char *dbname, const char *tblspcname)
 	/* Now it's safe to release the database lock */
 	UnlockSharedObjectForSession(DatabaseRelationId, db_id, 0,
 								 AccessExclusiveLock);
+
+	pfree(src_dbpath);
+	pfree(dst_dbpath);
 }
 
 /* Error cleanup callback for movedb */
@@ -2141,6 +2148,8 @@ movedb_failure_callback(int code, Datum arg)
 	dstpath = GetDatabasePath(fparms->dest_dboid, fparms->dest_tsoid);
 
 	(void) rmtree(dstpath, true);
+
+	pfree(dstpath);
 }
 
 /*
@@ -3051,6 +3060,9 @@ dbase_redo(XLogReaderState *record)
 		 * We don't need to copy subdirectories
 		 */
 		copydir(src_path, dst_path, false);
+
+		pfree(src_path);
+		pfree(dst_path);
 	}
 	else if (info == XLOG_DBASE_CREATE_WAL_LOG)
 	{
@@ -3063,6 +3075,7 @@ dbase_redo(XLogReaderState *record)
 		/* Create the database directory with the version file. */
 		CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,
 								true);
+		pfree(dbpath);
 	}
 	else if (info == XLOG_DBASE_DROP)
 	{
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
@@ -1057,6 +1057,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
 	}
 
 	SetDatabasePath(fullpath);
+	pfree(fullpath);
 
 	/*
 	 * It's now possible to do real access to the system catalogs.

Original file line number	Diff line number	Diff line change
`@@ -217,6 +217,8 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid,`
`217`	`217`	`UnlockRelationId(&dstrelid, AccessShareLock);`
`218`	`218`	`}`
`219`	`219`
	`220`	`+ pfree(srcpath);`
	`221`	`+ pfree(dstpath);`
`220`	`222`	`list_free_deep(rnodelist);`
`221`	`223`	`}`
`222`	`224`
`@@ -628,6 +630,8 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid,`
`628`	`630`	`(void) XLogInsert(RM_DBASE_ID,`
`629`	`631`	`XLOG_DBASE_CREATE_FILE_COPY \| XLR_SPECIAL_REL_UPDATE);`
`630`	`632`	`}`
	`633`	`+ pfree(srcpath);`
	`634`	`+ pfree(dstpath);`
`631`	`635`	`}`
`632`	`636`	`table_endscan(scan);`
`633`	`637`	`table_close(rel, AccessShareLock);`
`@@ -2128,6 +2132,9 @@ movedb(const char dbname, const char tblspcname)`
`2128`	`2132`	`/* Now it's safe to release the database lock */`
`2129`	`2133`	`UnlockSharedObjectForSession(DatabaseRelationId, db_id, 0,`
`2130`	`2134`	`AccessExclusiveLock);`
	`2135`	`+`
	`2136`	`+ pfree(src_dbpath);`
	`2137`	`+ pfree(dst_dbpath);`
`2131`	`2138`	`}`
`2132`	`2139`
`2133`	`2140`	`/* Error cleanup callback for movedb */`
`@@ -2141,6 +2148,8 @@ movedb_failure_callback(int code, Datum arg)`
`2141`	`2148`	`dstpath = GetDatabasePath(fparms->dest_dboid, fparms->dest_tsoid);`
`2142`	`2149`
`2143`	`2150`	`(void) rmtree(dstpath, true);`
	`2151`	`+`
	`2152`	`+ pfree(dstpath);`
`2144`	`2153`	`}`
`2145`	`2154`
`2146`	`2155`	`/*`
`@@ -3051,6 +3060,9 @@ dbase_redo(XLogReaderState *record)`
`3051`	`3060`	`* We don't need to copy subdirectories`
`3052`	`3061`	`*/`
`3053`	`3062`	`copydir(src_path, dst_path, false);`
	`3063`	`+`
	`3064`	`+ pfree(src_path);`
	`3065`	`+ pfree(dst_path);`
`3054`	`3066`	`}`
`3055`	`3067`	`else if (info == XLOG_DBASE_CREATE_WAL_LOG)`
`3056`	`3068`	`{`
`@@ -3063,6 +3075,7 @@ dbase_redo(XLogReaderState *record)`
`3063`	`3075`	`/* Create the database directory with the version file. */`
`3064`	`3076`	`CreateDirAndVersionFile(dbpath, xlrec->db_id, xlrec->tablespace_id,`
`3065`	`3077`	`true);`
	`3078`	`+ pfree(dbpath);`
`3066`	`3079`	`}`
`3067`	`3080`	`else if (info == XLOG_DBASE_DROP)`
`3068`	`3081`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1057,6 +1057,7 @@ InitPostgres(const char in_dbname, Oid dboid, const char username,`
`1057`	`1057`	`}`
`1058`	`1058`
`1059`	`1059`	`SetDatabasePath(fullpath);`
	`1060`	`+ pfree(fullpath);`
`1060`	`1061`
`1061`	`1062`	`/*`
`1062`	`1063`	`* It's now possible to do real access to the system catalogs.`