[PATCH 2/5] Make relpathbackend return a statically result instead of palloc()'ing it

From: Andres Freund <andres(at)anarazel(dot)de>

relpathbackend() (via some of its wrappers) is used in *_desc routines which we
want to be useable without a backend environment arround.

Change signature to return a 'const char *' to make misuse easier to
detect. That necessicates also changing the 'FileName' typedef to 'const char
*' which seems to be a good idea anyway.
---
src/backend/access/rmgrdesc/smgrdesc.c | 6 ++---
src/backend/access/rmgrdesc/xactdesc.c | 6 ++---
src/backend/access/transam/xlogutils.c | 9 +++----
src/backend/catalog/catalog.c | 49 +++++++++++-----------------------
src/backend/storage/buffer/bufmgr.c | 12 +++------
src/backend/storage/file/fd.c | 6 ++---
src/backend/storage/smgr/md.c | 23 +++++-----------
src/backend/utils/adt/dbsize.c | 4 +--
src/include/catalog/catalog.h | 2 +-
src/include/storage/fd.h | 9 +++----
10 files changed, 42 insertions(+), 84 deletions(-)

diff --git a/src/backend/access/rmgrdesc/smgrdesc.c b/src/backend/access/rmgrdesc/smgrdesc.c
index bcabf89..490c8c7 100644
--- a/src/backend/access/rmgrdesc/smgrdesc.c
+++ b/src/backend/access/rmgrdesc/smgrdesc.c
@@ -26,19 +26,17 @@ smgr_desc(StringInfo buf, uint8 xl_info, char *rec)
if (info == XLOG_SMGR_CREATE)
{
xl_smgr_create *xlrec = (xl_smgr_create *) rec;
- char *path = relpathperm(xlrec->rnode, xlrec->forkNum);
+ const char *path = relpathperm(xlrec->rnode, xlrec->forkNum);

appendStringInfo(buf, "file create: %s", path);
- pfree(path);
}
else if (info == XLOG_SMGR_TRUNCATE)
{
xl_smgr_truncate *xlrec = (xl_smgr_truncate *) rec;
- char *path = relpathperm(xlrec->rnode, MAIN_FORKNUM);
+ const char *path = relpathperm(xlrec->rnode, MAIN_FORKNUM);

appendStringInfo(buf, "file truncate: %s to %u blocks", path,
xlrec->blkno);
- pfree(path);
}
else
appendStringInfo(buf, "UNKNOWN");
diff --git a/src/backend/access/rmgrdesc/xactdesc.c b/src/backend/access/rmgrdesc/xactdesc.c
index 2471279..b86a53e 100644
--- a/src/backend/access/rmgrdesc/xactdesc.c
+++ b/src/backend/access/rmgrdesc/xactdesc.c
@@ -35,10 +35,9 @@ xact_desc_commit(StringInfo buf, xl_xact_commit *xlrec)
appendStringInfo(buf, "; rels:");
for (i = 0; i < xlrec->nrels; i++)
{
- char *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
+ const char *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);

appendStringInfo(buf, " %s", path);
- pfree(path);
}
}
if (xlrec->nsubxacts > 0)
@@ -105,10 +104,9 @@ xact_desc_abort(StringInfo buf, xl_xact_abort *xlrec)
appendStringInfo(buf, "; rels:");
for (i = 0; i < xlrec->nrels; i++)
{
- char *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);
+ const char *path = relpathperm(xlrec->xnodes[i], MAIN_FORKNUM);

appendStringInfo(buf, " %s", path);
- pfree(path);
}
}
if (xlrec->nsubxacts > 0)
diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index f9a6e62..8266f3c 100644
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -57,7 +57,7 @@ static void
report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
BlockNumber blkno, bool present)
{
- char *path = relpathperm(node, forkno);
+ const char *path = relpathperm(node, forkno);

if (present)
elog(elevel, "page %u of relation %s is uninitialized",
@@ -65,7 +65,6 @@ report_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
else
elog(elevel, "page %u of relation %s does not exist",
blkno, path);
- pfree(path);
}

/* Log a reference to an invalid page */
@@ -153,11 +152,10 @@ forget_invalid_pages(RelFileNode node, ForkNumber forkno, BlockNumber minblkno)
{
if (log_min_messages <= DEBUG2 || client_min_messages <= DEBUG2)
{
- char *path = relpathperm(hentry->key.node, forkno);
+ const char *path = relpathperm(hentry->key.node, forkno);

elog(DEBUG2, "page %u of relation %s has been dropped",
hentry->key.blkno, path);
- pfree(path);
}

if (hash_search(invalid_page_tab,
@@ -186,11 +184,10 @@ forget_invalid_pages_db(Oid dbid)
{
if (log_min_messages <= DEBUG2 || client_min_messages <= DEBUG2)
{
- char *path = relpathperm(hentry->key.node, hentry->key.forkno);
+ const char *path = relpathperm(hentry->key.node, hentry->key.forkno);

elog(DEBUG2, "page %u of relation %s has been dropped",
hentry->key.blkno, path);
- pfree(path);
}

if (hash_search(invalid_page_tab,
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index 9686486..6455ef0 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -112,54 +112,45 @@ forkname_chars(const char *str, ForkNumber *fork)
/*
* relpathbackend - construct path to a relation's file
*
- * Result is a palloc'd string.
+ * Result is a pointer to a statically allocated string.
*/
-char *
+const char *
relpathbackend(RelFileNode rnode, BackendId backend, ForkNumber forknum)
{
- int pathlen;
- char *path;
+ static char path[MAXPGPATH];

if (rnode.spcNode == GLOBALTABLESPACE_OID)
{
/* Shared system relations live in {datadir}/global */
Assert(rnode.dbNode == 0);
Assert(backend == InvalidBackendId);
- pathlen = 7 + OIDCHARS + 1 + FORKNAMECHARS + 1;
- path = (char *) palloc(pathlen);
if (forknum != MAIN_FORKNUM)
- snprintf(path, pathlen, "global/%u_%s",
+ snprintf(path, MAXPGPATH, "global/%u_%s",
rnode.relNode, forkNames[forknum]);
else
- snprintf(path, pathlen, "global/%u", rnode.relNode);
+ snprintf(path, MAXPGPATH, "global/%u", rnode.relNode);
}
else if (rnode.spcNode == DEFAULTTABLESPACE_OID)
{
/* The default tablespace is {datadir}/base */
if (backend == InvalidBackendId)
{
- pathlen = 5 + OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1;
- path = (char *) palloc(pathlen);
if (forknum != MAIN_FORKNUM)
- snprintf(path, pathlen, "base/%u/%u_%s",
+ snprintf(path, MAXPGPATH, "base/%u/%u_%s",
rnode.dbNode, rnode.relNode,
forkNames[forknum]);
else
- snprintf(path, pathlen, "base/%u/%u",
+ snprintf(path, MAXPGPATH, "base/%u/%u",
rnode.dbNode, rnode.relNode);
}
else
{
- /* OIDCHARS will suffice for an integer, too */
- pathlen = 5 + OIDCHARS + 2 + OIDCHARS + 1 + OIDCHARS + 1
- + FORKNAMECHARS + 1;
- path = (char *) palloc(pathlen);
if (forknum != MAIN_FORKNUM)
- snprintf(path, pathlen, "base/%u/t%d_%u_%s",
+ snprintf(path, MAXPGPATH, "base/%u/t%d_%u_%s",
rnode.dbNode, backend, rnode.relNode,
forkNames[forknum]);
else
- snprintf(path, pathlen, "base/%u/t%d_%u",
+ snprintf(path, MAXPGPATH, "base/%u/t%d_%u",
rnode.dbNode, backend, rnode.relNode);
}
}
@@ -168,38 +159,30 @@ relpathbackend(RelFileNode rnode, BackendId backend, ForkNumber forknum)
/* All other tablespaces are accessed via symlinks */
if (backend == InvalidBackendId)
{
- pathlen = 9 + 1 + OIDCHARS + 1
- + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 + OIDCHARS + 1
- + OIDCHARS + 1 + FORKNAMECHARS + 1;
- path = (char *) palloc(pathlen);
if (forknum != MAIN_FORKNUM)
- snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/%u_%s",
+ snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%u/%u_%s",
rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
rnode.dbNode, rnode.relNode,
forkNames[forknum]);
else
- snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/%u",
+ snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%u/%u",
rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
rnode.dbNode, rnode.relNode);
}
else
{
- /* OIDCHARS will suffice for an integer, too */
- pathlen = 9 + 1 + OIDCHARS + 1
- + strlen(TABLESPACE_VERSION_DIRECTORY) + 1 + OIDCHARS + 2
- + OIDCHARS + 1 + OIDCHARS + 1 + FORKNAMECHARS + 1;
- path = (char *) palloc(pathlen);
if (forknum != MAIN_FORKNUM)
- snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/t%d_%u_%s",
+ snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%u/t%d_%u_%s",
rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
rnode.dbNode, backend, rnode.relNode,
forkNames[forknum]);
else
- snprintf(path, pathlen, "pg_tblspc/%u/%s/%u/t%d_%u",
+ snprintf(path, MAXPGPATH, "pg_tblspc/%u/%s/%u/t%d_%u",
rnode.spcNode, TABLESPACE_VERSION_DIRECTORY,
rnode.dbNode, backend, rnode.relNode);
}
}
+
return path;
}

@@ -534,7 +517,7 @@ Oid
GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
{
RelFileNodeBackend rnode;
- char *rpath;
+ const char *rpath;
int fd;
bool collides;
BackendId backend;
@@ -599,8 +582,6 @@ GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence)
*/
collides = false;
}
-
- pfree(rpath);
} while (collides);

return rnode.node.relNode;
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 03ed41d..6c2620d 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1757,7 +1757,7 @@ PrintBufferLeakWarning(Buffer buffer)
{
volatile BufferDesc *buf;
int32 loccount;
- char *path;
+ const char *path;
BackendId backend;

Assert(BufferIsValid(buffer));
@@ -1782,7 +1782,6 @@ PrintBufferLeakWarning(Buffer buffer)
buffer, path,
buf->tag.blockNum, buf->flags,
buf->refcount, loccount);
- pfree(path);
}

/*
@@ -2901,7 +2900,7 @@ AbortBufferIO(void)
if (sv_flags & BM_IO_ERROR)
{
/* Buffer is pinned, so we can read tag without spinlock */
- char *path;
+ const char *path;

path = relpathperm(buf->tag.rnode, buf->tag.forkNum);
ereport(WARNING,
@@ -2909,7 +2908,6 @@ AbortBufferIO(void)
errmsg("could not write block %u of %s",
buf->tag.blockNum, path),
errdetail("Multiple failures --- write error might be permanent.")));
- pfree(path);
}
}
TerminateBufferIO(buf, false, BM_IO_ERROR);
@@ -2927,11 +2925,10 @@ shared_buffer_write_error_callback(void *arg)
/* Buffer is pinned, so we can read the tag without locking the spinlock */
if (bufHdr != NULL)
{
- char *path = relpathperm(bufHdr->tag.rnode, bufHdr->tag.forkNum);
+ const char *path = relpathperm(bufHdr->tag.rnode, bufHdr->tag.forkNum);

errcontext("writing block %u of relation %s",
bufHdr->tag.blockNum, path);
- pfree(path);
}
}

@@ -2945,11 +2942,10 @@ local_buffer_write_error_callback(void *arg)

if (bufHdr != NULL)
{
- char *path = relpathbackend(bufHdr->tag.rnode, MyBackendId,
+ const char *path = relpathbackend(bufHdr->tag.rnode, MyBackendId,
bufHdr->tag.forkNum);

errcontext("writing block %u of relation %s",
bufHdr->tag.blockNum, path);
- pfree(path);
}
}
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index c026731..64d2a1e 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -556,7 +556,7 @@ set_max_safe_fds(void)
* this module wouldn't have any open files to close at that point anyway.
*/
int
-BasicOpenFile(FileName fileName, int fileFlags, int fileMode)
+BasicOpenFile(const char *fileName, int fileFlags, int fileMode)
{
int fd;

@@ -878,7 +878,7 @@ FileInvalidate(File file)
* (which should always be $PGDATA when this code is running).
*/
File
-PathNameOpenFile(FileName fileName, int fileFlags, int fileMode)
+PathNameOpenFile(const char *fileName, int fileFlags, int fileMode)
{
char *fnamecopy;
File file;
@@ -1548,7 +1548,7 @@ TryAgain:
* Like AllocateFile, but returns an unbuffered fd like open(2)
*/
int
-OpenTransientFile(FileName fileName, int fileFlags, int fileMode)
+OpenTransientFile(const char *fileName, int fileFlags, int fileMode)
{
int fd;

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 20eb36a..4ef47b1 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -264,7 +264,7 @@ mdexists(SMgrRelation reln, ForkNumber forkNum)
void
mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
{
- char *path;
+ const char *path;
File fd;

if (isRedo && reln->md_fd[forkNum] != NULL)
@@ -298,8 +298,6 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
}
}

- pfree(path);
-
reln->md_fd[forkNum] = _fdvec_alloc();

reln->md_fd[forkNum]->mdfd_vfd = fd;
@@ -380,7 +378,7 @@ mdunlink(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
static void
mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
{
- char *path;
+ const char *path;
int ret;

path = relpath(rnode, forkNum);
@@ -449,8 +447,6 @@ mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
}
pfree(segpath);
}
-
- pfree(path);
}

/*
@@ -545,7 +541,7 @@ static MdfdVec *
mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
{
MdfdVec *mdfd;
- char *path;
+ const char *path;
File fd;

/* No work if already open */
@@ -571,7 +567,6 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
if (behavior == EXTENSION_RETURN_NULL &&
FILE_POSSIBLY_DELETED(errno))
{
- pfree(path);
return NULL;
}
ereport(ERROR,
@@ -580,8 +575,6 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
}
}

- pfree(path);
-
reln->md_fd[forknum] = mdfd = _fdvec_alloc();

mdfd->mdfd_vfd = fd;
@@ -1279,7 +1272,7 @@ mdpostckpt(void)
while (pendingUnlinks != NIL)
{
PendingUnlinkEntry *entry = (PendingUnlinkEntry *) linitial(pendingUnlinks);
- char *path;
+ const char *path;

/*
* New entries are appended to the end, so if the entry is new we've
@@ -1309,7 +1302,6 @@ mdpostckpt(void)
(errcode_for_file_access(),
errmsg("could not remove file \"%s\": %m", path)));
}
- pfree(path);

/* And remove the list entry */
pendingUnlinks = list_delete_first(pendingUnlinks);
@@ -1634,8 +1626,8 @@ _fdvec_alloc(void)
static char *
_mdfd_segpath(SMgrRelation reln, ForkNumber forknum, BlockNumber segno)
{
- char *path,
- *fullpath;
+ const char *path;
+ char *fullpath;

path = relpath(reln->smgr_rnode, forknum);

@@ -1644,10 +1636,9 @@ _mdfd_segpath(SMgrRelation reln, ForkNumber forknum, BlockNumber segno)
/* be sure we have enough space for the '.segno' */
fullpath = (char *) palloc(strlen(path) + 12);
sprintf(fullpath, "%s.%u", path, segno);
- pfree(path);
}
else
- fullpath = path;
+ fullpath = pstrdup(path);

return fullpath;
}
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 89ad386..c285007 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -268,7 +268,7 @@ static int64
calculate_relation_size(RelFileNode *rfn, BackendId backend, ForkNumber forknum)
{
int64 totalsize = 0;
- char *relationpath;
+ const char *relationpath;
char pathname[MAXPGPATH];
unsigned int segcount = 0;

@@ -756,7 +756,7 @@ pg_relation_filepath(PG_FUNCTION_ARGS)
Form_pg_class relform;
RelFileNode rnode;
BackendId backend;
- char *path;
+ const char *path;

tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(tuple))
diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h
index 5d506fe..299cb03 100644
--- a/src/include/catalog/catalog.h
+++ b/src/include/catalog/catalog.h
@@ -31,7 +31,7 @@ extern const char *forkNames[];
extern ForkNumber forkname_to_number(char *forkName);
extern int forkname_chars(const char *str, ForkNumber *);

-extern char *relpathbackend(RelFileNode rnode, BackendId backend,
+extern const char *relpathbackend(RelFileNode rnode, BackendId backend,
ForkNumber forknum);
extern char *GetDatabasePath(Oid dbNode, Oid spcNode);

diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index bd36c9d..b2565c4 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -45,9 +45,6 @@
/*
* FileSeek uses the standard UNIX lseek(2) flags.
*/
-
-typedef char *FileName;
-
typedef int File;


@@ -65,7 +62,7 @@ extern int max_safe_fds;
*/

/* Operations on virtual Files --- equivalent to Unix kernel file ops */
-extern File PathNameOpenFile(FileName fileName, int fileFlags, int fileMode);
+extern File PathNameOpenFile(const char *fileName, int fileFlags, int fileMode);
extern File OpenTemporaryFile(bool interXact);
extern void FileClose(File file);
extern int FilePrefetch(File file, off_t offset, int amount);
@@ -86,11 +83,11 @@ extern struct dirent *ReadDir(DIR *dir, const char *dirname);
extern int FreeDir(DIR *dir);

/* Operations to allow use of a plain kernel FD, with automatic cleanup */
-extern int OpenTransientFile(FileName fileName, int fileFlags, int fileMode);
+extern int OpenTransientFile(const char *fileName, int fileFlags, int fileMode);
extern int CloseTransientFile(int fd);

/* If you've really really gotta have a plain kernel FD, use this */
-extern int BasicOpenFile(FileName fileName, int fileFlags, int fileMode);
+extern int BasicOpenFile(const char *fileName, int fileFlags, int fileMode);

/* Miscellaneous support routines */
extern void InitFileAccess(void);
--
1.7.12.289.g0ce9864.dirty