On Sun, Apr 3, 2022 at 9:52 PM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2022-03-29 11:55:05 -0400, Robert Haas wrote:
> > I committed v6 instead.
>
> Coverity complains that this patch added GetDatabasePath() calls without
> freeing its return value. Normally that'd be easy to dismiss, due to memory
> contexts, but there's no granular resets in CreateDatabaseUsingFileCopy(). And
> obviously there can be a lot of relations in one database - we shouldn't hold
> onto the same path over and over again.
> The case in recovery is worse, because there we don't have a memory context to
> reset afaics. Oddly enough, it sure looks like we have an existing version of
> this bug in the file-copy path?
Yeah, I see that the createdb() and dbase_redo() had this existing
problem and with this patch we have created a few more such
occurrences.
The attached patch fixes it.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index df16533..ff81c48 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -218,6 +218,8 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid,
}
list_free_deep(rnodelist);
+ pfree(srcpath);
+ pfree(dstpath);
}
/*
@@ -628,6 +630,9 @@ 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);
@@ -3051,6 +3056,8 @@ 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 +3070,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)
{
