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

From: Andres Freund <andres@anarazel.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
%ublocks", 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 voidreport_invalid_page(int elevel, RelFileNode node, ForkNumber forkno,
BlockNumberblkno, 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'sfile *
 
- * 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
livein {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
tablespacesare 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 @@ OidGetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence){
RelFileNodeBackendrnode;
 
-    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
ispinned, 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
bepermanent.")));
 
-                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
withoutlocking 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)voidmdcreate(SMgrRelation reln, ForkNumber forkNum,
boolisRedo){
 
-    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
voidmdunlinkfork(RelFileNodeBackendrnode, 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("couldnot 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
haveenough 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 int64calculate_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(constchar *str, ForkNumber *);
 
-extern char *relpathbackend(RelFileNode rnode, BackendId backend,
+extern const char *relpathbackend(RelFileNode rnode, BackendId backend,               ForkNumber forknum);extern char
*GetDatabasePath(OiddbNode, 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);externvoid 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);/*
Operationsto 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(intfd);/* 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
*/externvoid InitFileAccess(void);
 
-- 
1.7.12.289.g0ce9864.dirty