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.

diff --git a/src/backend/postmaster/syslogger.c b/src/backend/postmaster/syslogger.c
index 39006466b2db66a850c40622b5aade920ddc9fff..198ca26324705f92e2c8ee35001aac7325c5972a 100644 (file)
--- 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.44.2.1 2009/03/18 08:44:52 heikki Exp $
+ *       $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.44.2.2 2009/11/19 02:45:50 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);
@@ -1017,6 +1017,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;
@@ -1027,17 +1028,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)
@@ -1085,7 +1081,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;
@@ -1101,8 +1099,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
@@ -1111,6 +1111,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. */
@@ -1147,7 +1148,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;
                }
 
@@ -1161,8 +1166,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
@@ -1171,8 +1178,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();
 }
 
@@ -1180,10 +1193,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;
@@ -1212,7 +1228,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;