Fix memory leak in syslogger: logfile_rotate() would leak a copy of the

output filename if CSV logging was enabled and only one of the two possible
output files got rotated during a particular call (which would, in fact,
typically be the case during a size-based rotation).  This would amount to
about MAXPGPATH (1KB) per rotation, and it's been there since the CSV
code was put in, so it's surprising that nobody noticed it before.
Per bug #5196 from Thomas Poindessous.

1 files changed, 33 insertions, 17 deletions

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index e6577c944d..89f2beb6ac 100644
--- a/src/backend/postmaster/syslogger.c
+++ b/src/backend/postmaster/syslogger.c
@@ -18,7 +18,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.51 2009/06/11 14:49:01 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.51.2.1 2009/11/19 02:45:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -140,7 +140,7 @@ static void open_csvlogfile(void);
 static unsigned int __stdcall pipeThread(void *arg);
 #endif
 static void logfile_rotate(bool time_based_rotation, int size_rotation_for);
-static char *logfile_getname(pg_time_t timestamp, char *suffix);
+static char *logfile_getname(pg_time_t timestamp, const char *suffix);
 static void set_next_rotation_time(void);
 static void sigHupHandler(SIGNAL_ARGS);
 static void sigUsr1Handler(SIGNAL_ARGS);
@@ -1016,6 +1016,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 {
 	char	   *filename;
 	char	   *csvfilename = NULL;
+	pg_time_t	fntime;
 	FILE	   *fh;
 
 	rotation_requested = false;
@@ -1026,17 +1027,12 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 	 * file name when we don't do the rotation immediately.
 	 */
 	if (time_based_rotation)
-	{
-		filename = logfile_getname(next_rotation_time, NULL);
-		if (csvlogFile != NULL)
-			csvfilename = logfile_getname(next_rotation_time, ".csv");
-	}
+		fntime = next_rotation_time;
 	else
-	{
-		filename = logfile_getname(time(NULL), NULL);
-		if (csvlogFile != NULL)
-			csvfilename = logfile_getname(time(NULL), ".csv");
-	}
+		fntime = time(NULL);
+	filename = logfile_getname(fntime, NULL);
+	if (csvlogFile != NULL)
+		csvfilename = logfile_getname(fntime, ".csv");
 
 	/*
 	 * Decide whether to overwrite or append.  We can overwrite if (a)
@@ -1084,7 +1080,9 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 				Log_RotationAge = 0;
 				Log_RotationSize = 0;
 			}
-			pfree(filename);
+
+			if (filename)
+				pfree(filename);
 			if (csvfilename)
 				pfree(csvfilename);
 			return;
@@ -1100,8 +1098,10 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 #ifdef WIN32
 		EnterCriticalSection(&sysfileSection);
 #endif
+
 		fclose(syslogFile);
 		syslogFile = fh;
+
 #ifdef WIN32
 		LeaveCriticalSection(&sysfileSection);
 #endif
@@ -1110,6 +1110,7 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (last_file_name != NULL)
 			pfree(last_file_name);
 		last_file_name = filename;
+		filename = NULL;
 	}
 
 	/* Same as above, but for csv file. */
@@ -1146,7 +1147,11 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 				Log_RotationAge = 0;
 				Log_RotationSize = 0;
 			}
-			pfree(csvfilename);
+
+			if (filename)
+				pfree(filename);
+			if (csvfilename)
+				pfree(csvfilename);
 			return;
 		}
 
@@ -1160,8 +1165,10 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 #ifdef WIN32
 		EnterCriticalSection(&sysfileSection);
 #endif
+
 		fclose(csvlogFile);
 		csvlogFile = fh;
+
 #ifdef WIN32
 		LeaveCriticalSection(&sysfileSection);
 #endif
@@ -1170,8 +1177,14 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (last_csv_file_name != NULL)
 			pfree(last_csv_file_name);
 		last_csv_file_name = csvfilename;
+		csvfilename = NULL;
 	}
 
+	if (filename)
+		pfree(filename);
+	if (csvfilename)
+		pfree(csvfilename);
+
 	set_next_rotation_time();
 }
 
@@ -1179,10 +1192,13 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 /*
  * construct logfile name using timestamp information
  *
+ * If suffix isn't NULL, append it to the name, replacing any ".log"
+ * that may be in the pattern.
+ *
  * Result is palloc'd.
  */
 static char *
-logfile_getname(pg_time_t timestamp, char *suffix)
+logfile_getname(pg_time_t timestamp, const char *suffix)
 {
 	char	   *filename;
 	int			len;
@@ -1193,7 +1209,7 @@ logfile_getname(pg_time_t timestamp, char *suffix)
 
 	len = strlen(filename);
 
-	/* treat it as a strftime pattern */
+	/* treat Log_filename as a strftime pattern */
 	pg_strftime(filename + len, MAXPGPATH - len, Log_filename,
 				pg_localtime(&timestamp, log_timezone));
 
@@ -1202,7 +1218,7 @@ logfile_getname(pg_time_t timestamp, char *suffix)
 		len = strlen(filename);
 		if (len > 4 && (strcmp(filename + (len - 4), ".log") == 0))
 			len -= 4;
-		strncpy(filename + len, suffix, MAXPGPATH - len);
+		strlcpy(filename + len, suffix, MAXPGPATH - len);
 	}
 
 	return filename;