Replace strncpy with strlcpy in selected places that seem possibly relevant

to performance.  (A wholesale effort to get rid of strncpy should be
undertaken sometime, but not during beta.)  This commit also fixes dynahash.c
to correctly truncate overlength string keys for hashtables, so that its
callers don't have to anymore.

diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index cab170a2330091dc1eb2f97f7234095bb9be128d..d722c93202a8128dce1a14e4aa75ba5612347072 100644 (file)
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -314,7 +314,6 @@ StorePreparedStatement(const char *stmt_name,
        MemoryContext oldcxt,
                                entrycxt;
        char       *qstring;
-       char            key[NAMEDATALEN];
        bool            found;
 
        /* Initialize the hash table, if necessary */
@@ -322,10 +321,7 @@ StorePreparedStatement(const char *stmt_name,
                InitQueryHashTable();
 
        /* Check for pre-existing entry of same name */
-       /* See notes in FetchPreparedStatement */
-       StrNCpy(key, stmt_name, sizeof(key));
-
-       hash_search(prepared_queries, key, HASH_FIND, &found);
+       hash_search(prepared_queries, stmt_name, HASH_FIND, &found);
 
        if (found)
                ereport(ERROR,
@@ -355,7 +351,7 @@ StorePreparedStatement(const char *stmt_name,
 
        /* Now we can add entry to hash table */
        entry = (PreparedStatement *) hash_search(prepared_queries,
-                                                                                         key,
+                                                                                         stmt_name,
                                                                                          HASH_ENTER,
                                                                                          &found);
 
@@ -384,7 +380,6 @@ StorePreparedStatement(const char *stmt_name,
 PreparedStatement *
 FetchPreparedStatement(const char *stmt_name, bool throwError)
 {
-       char            key[NAMEDATALEN];
        PreparedStatement *entry;
 
        /*
@@ -392,19 +387,10 @@ FetchPreparedStatement(const char *stmt_name, bool throwError)
         * anything, therefore it couldn't possibly store our plan.
         */
        if (prepared_queries)
-       {
-               /*
-                * We can't just use the statement name as supplied by the user: the
-                * hash package is picky enough that it needs to be NUL-padded out to
-                * the appropriate length to work correctly.
-                */
-               StrNCpy(key, stmt_name, sizeof(key));
-
                entry = (PreparedStatement *) hash_search(prepared_queries,
-                                                                                                 key,
+                                                                                                 stmt_name,
                                                                                                  HASH_FIND,
                                                                                                  NULL);
-       }
        else
                entry = NULL;
 

diff --git a/src/backend/nodes/read.c b/src/backend/nodes/read.c
index 82b39698f688e543afce247281bde4459882a133..54a261023fab938e44d63897b353eb066c36f2e3 100644 (file)
--- a/src/backend/nodes/read.c
+++ b/src/backend/nodes/read.c
@@ -408,7 +408,7 @@ nodeRead(char *token, int tok_len)
                                char       *val = palloc(tok_len);
 
                                /* skip leading 'b' */
-                               strncpy(val, token + 1, tok_len - 1);
+                               memcpy(val, token + 1, tok_len - 1);
                                val[tok_len - 1] = '\0';
                                result = (Node *) makeBitString(val);
                                break;

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index d44411ff244b59dc09c7cddd31fc3376b4fd1e09..e98b5c4efbeed06ab8b4e5111965c92a633c8c70 100644 (file)
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -456,13 +456,9 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
 void *
 ShmemInitStruct(const char *name, Size size, bool *foundPtr)
 {
-       ShmemIndexEnt *result,
-                               item;
+       ShmemIndexEnt *result;
        void       *structPtr;
 
-       strncpy(item.key, name, SHMEM_INDEX_KEYSIZE);
-       item.location = BAD_LOCATION;
-
        LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
 
        if (!ShmemIndex)
@@ -498,7 +494,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
 
        /* look it up in the shmem index */
        result = (ShmemIndexEnt *)
-               hash_search(ShmemIndex, (void *) &item, HASH_ENTER_NULL, foundPtr);
+               hash_search(ShmemIndex, name, HASH_ENTER_NULL, foundPtr);
 
        if (!result)
        {
@@ -533,12 +529,13 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
                {
                        /* out of memory */
                        Assert(ShmemIndex);
-                       hash_search(ShmemIndex, (void *) &item, HASH_REMOVE, NULL);
+                       hash_search(ShmemIndex, name, HASH_REMOVE, NULL);
                        LWLockRelease(ShmemIndexLock);
 
                        ereport(WARNING,
                                        (errcode(ERRCODE_OUT_OF_MEMORY),
-                       errmsg("could not allocate shared memory segment \"%s\"", name)));
+                                        errmsg("could not allocate shared memory segment \"%s\"",
+                                                       name)));
                        *foundPtr = FALSE;
                        return NULL;
                }

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 2f6a34556b8132d14b4c07959bef947ad2480238..34bfeb27dcb951d4f6be164448bef73eaee67b5f 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1225,6 +1225,7 @@ write_syslog(int level, const char *line)
                while (len > 0)
                {
                        char            buf[PG_SYSLOG_LIMIT + 1];
+                       const char *nlpos;
                        int                     buflen;
                        int                     i;
 
@@ -1236,12 +1237,15 @@ write_syslog(int level, const char *line)
                                continue;
                        }
 
-                       strncpy(buf, line, PG_SYSLOG_LIMIT);
-                       buf[PG_SYSLOG_LIMIT] = '\0';
-                       if (strchr(buf, '\n') != NULL)
-                               *strchr(buf, '\n') = '\0';
-
-                       buflen = strlen(buf);
+                       /* copy one line, or as much as will fit, to buf */
+                       nlpos = strchr(line, '\n');
+                       if (nlpos != NULL)
+                               buflen = nlpos - line;
+                       else
+                               buflen = len;
+                       buflen = Min(buflen, PG_SYSLOG_LIMIT);
+                       memcpy(buf, line, buflen);
+                       buf[buflen] = '\0';
 
                        /* trim to multibyte letter boundary */
                        buflen = pg_mbcliplen(buf, buflen, buflen);

diff --git a/src/backend/utils/fmgr/dfmgr.c b/src/backend/utils/fmgr/dfmgr.c
index fd3d9f5bdfaf0325aa21e5e6d3eb3444561f9f1c..e3f3c11ce3c521905840e0760ae02111af7f9137 100644 (file)
--- a/src/backend/utils/fmgr/dfmgr.c
+++ b/src/backend/utils/fmgr/dfmgr.c
@@ -594,7 +594,6 @@ find_rendezvous_variable(const char *varName)
 {
        static HTAB         *rendezvousHash = NULL;
 
-       char                         key[NAMEDATALEN];
        rendezvousHashEntry *hentry;
        bool                             found;
 
@@ -612,12 +611,9 @@ find_rendezvous_variable(const char *varName)
                                                                         HASH_ELEM);
        }
 
-       /* Turn the varName into a fixed-size string */
-       StrNCpy(key, varName, sizeof(key));
-
        /* Find or create the hashtable entry for this varName */
        hentry = (rendezvousHashEntry *) hash_search(rendezvousHash,
-                                                                                                key,
+                                                                                                varName,
                                                                                                 HASH_ENTER,
                                                                                                 &found);
 

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 99e419c241bed6ed8b5a98727d6e11ca5f4085d3..d66bd9c33ca9319541c3c012f9d298b2fe7de82f 100644 (file)
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -209,6 +209,20 @@ DynaHashAlloc(Size size)
 }
 
 
+/*
+ * HashCompareFunc for string keys
+ *
+ * Because we copy keys with strlcpy(), they will be truncated at keysize-1
+ * bytes, so we can only compare that many ... hence strncmp is almost but
+ * not quite the right thing.
+ */
+static int
+string_compare(const char *key1, const char *key2, Size keysize)
+{
+       return strncmp(key1, key2, keysize - 1);
+}
+
+
 /************************** CREATE ROUTINES **********************/
 
 /*
@@ -273,24 +287,24 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
                hashp->hash = string_hash;              /* default hash function */
 
        /*
-        * If you don't specify a match function, it defaults to strncmp() if you
-        * used string_hash (either explicitly or by default) and to memcmp()
-        * otherwise.  (Prior to PostgreSQL 7.4, memcmp() was always used.)
+        * If you don't specify a match function, it defaults to string_compare
+        * if you used string_hash (either explicitly or by default) and to memcmp
+        * otherwise.  (Prior to PostgreSQL 7.4, memcmp was always used.)
         */
        if (flags & HASH_COMPARE)
                hashp->match = info->match;
        else if (hashp->hash == string_hash)
-               hashp->match = (HashCompareFunc) strncmp;
+               hashp->match = (HashCompareFunc) string_compare;
        else
                hashp->match = memcmp;
 
        /*
-        * Similarly, the key-copying function defaults to strncpy() or memcpy().
+        * Similarly, the key-copying function defaults to strlcpy or memcpy.
         */
        if (flags & HASH_KEYCOPY)
                hashp->keycopy = info->keycopy;
        else if (hashp->hash == string_hash)
-               hashp->keycopy = (HashCopyFunc) strncpy;
+               hashp->keycopy = (HashCopyFunc) strlcpy;
        else
                hashp->keycopy = memcpy;
 

diff --git a/src/backend/utils/hash/hashfn.c b/src/backend/utils/hash/hashfn.c
index 2a2a35149909b402db45fb73141c5d48a09168fc..d1428a95e2738f00c21e1ef1a9ba89ec6d1a00ff 100644 (file)
--- a/src/backend/utils/hash/hashfn.c
+++ b/src/backend/utils/hash/hashfn.c
@@ -27,8 +27,16 @@
 uint32
 string_hash(const void *key, Size keysize)
 {
+       /*
+        * If the string exceeds keysize-1 bytes, we want to hash only that many,
+        * because when it is copied into the hash table it will be truncated at
+        * that length.
+        */
+       Size    s_len = strlen((const char *) key);
+
+       s_len = Min(s_len, keysize-1);
        return DatumGetUInt32(hash_any((const unsigned char *) key,
-                                                                  (int) strlen((const char *) key)));
+                                                                  (int) s_len));
 }
 
 /*

diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index 5ad67970743ff01b76eda59fa0f861f9bced8b0a..31a12e42b75b7a0a4256481f4dc061fab1e3cb99 100644 (file)
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -300,7 +300,7 @@ set_ps_display(const char *activity, bool force)
 #endif
 
        /* Update ps_buffer to contain both fixed part and activity */
-       StrNCpy(ps_buffer + ps_buffer_fixed_size, activity,
+       strlcpy(ps_buffer + ps_buffer_fixed_size, activity,
                        ps_buffer_size - ps_buffer_fixed_size);
 
        /* Transmit new setting to kernel, if necessary */

diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 87c5d408ac3256e80be827b46035301eb2e2fce5..68e3ce2ac2d5e495d4937a574d7d90370e6d22ab 100644 (file)
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -54,12 +54,10 @@ static HTAB *PortalHashTable = NULL;
 
 #define PortalHashTableLookup(NAME, PORTAL) \
 do { \
-       PortalHashEnt *hentry; char key[MAX_PORTALNAME_LEN]; \
+       PortalHashEnt *hentry; \
        \
-       MemSet(key, 0, MAX_PORTALNAME_LEN); \
-       StrNCpy(key, NAME, MAX_PORTALNAME_LEN); \
        hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
-                                                                                  key, HASH_FIND, NULL);       \
+                                                                                  (NAME), HASH_FIND, NULL); \
        if (hentry) \
                PORTAL = hentry->portal; \
        else \
@@ -68,12 +66,10 @@ do { \
 
 #define PortalHashTableInsert(PORTAL, NAME) \
 do { \
-       PortalHashEnt *hentry; bool found; char key[MAX_PORTALNAME_LEN]; \
+       PortalHashEnt *hentry; bool found; \
        \
-       MemSet(key, 0, MAX_PORTALNAME_LEN); \
-       StrNCpy(key, NAME, MAX_PORTALNAME_LEN); \
        hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
-                                                                                  key, HASH_ENTER, &found);    \
+                                                                                  (NAME), HASH_ENTER, &found); \
        if (found) \
                elog(ERROR, "duplicate portal name"); \
        hentry->portal = PORTAL; \
@@ -83,12 +79,10 @@ do { \
 
 #define PortalHashTableDelete(PORTAL) \
 do { \
-       PortalHashEnt *hentry; char key[MAX_PORTALNAME_LEN]; \
+       PortalHashEnt *hentry; \
        \
-       MemSet(key, 0, MAX_PORTALNAME_LEN); \
-       StrNCpy(key, PORTAL->name, MAX_PORTALNAME_LEN); \
        hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
-                                                                                  key, HASH_REMOVE, NULL); \
+                                                                                  PORTAL->name, HASH_REMOVE, NULL); \
        if (hentry == NULL) \
                elog(WARNING, "trying to delete portal name that does not exist"); \
 } while(0)

diff --git a/src/port/path.c b/src/port/path.c
index 355788e9d66e556678274644d13429f0e98f2fdf..c169fe3b1fbb7203fdc405b5fcc332e4abab1d62 100644 (file)
--- a/src/port/path.c
+++ b/src/port/path.c
@@ -174,7 +174,7 @@ join_path_components(char *ret_path,
                                         const char *head, const char *tail)
 {
        if (ret_path != head)
-               StrNCpy(ret_path, head, MAXPGPATH);
+               strlcpy(ret_path, head, MAXPGPATH);
 
        /*
         * Remove any leading "." and ".." in the tail component, adjusting head
@@ -493,7 +493,7 @@ make_relative_path(char *ret_path, const char *target_path,
         * Set up my_exec_path without the actual executable name, and
         * canonicalize to simplify comparison to bin_path.
         */
-       StrNCpy(ret_path, my_exec_path, MAXPGPATH);
+       strlcpy(ret_path, my_exec_path, MAXPGPATH);
        trim_directory(ret_path);       /* remove my executable name */
        canonicalize_path(ret_path);
 
@@ -513,7 +513,7 @@ make_relative_path(char *ret_path, const char *target_path,
        }
 
 no_match:
-       StrNCpy(ret_path, target_path, MAXPGPATH);
+       strlcpy(ret_path, target_path, MAXPGPATH);
        canonicalize_path(ret_path);
 }
 
@@ -625,7 +625,7 @@ get_home_path(char *ret_path)
 
        if (pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) != 0)
                return false;
-       StrNCpy(ret_path, pwd->pw_dir, MAXPGPATH);
+       strlcpy(ret_path, pwd->pw_dir, MAXPGPATH);
        return true;
 #else
        char            tmppath[MAX_PATH];

diff --git a/src/port/thread.c b/src/port/thread.c
index 1c5c847d2987fbff6366af9be26accfadbb6d038..1971a415b9efad990e8877e17978436aa851c3db 100644 (file)
--- a/src/port/thread.c
+++ b/src/port/thread.c
@@ -81,7 +81,7 @@ pqStrerror(int errnum, char *strerrbuf, size_t buflen)
 #endif
 #else
        /* no strerror_r() available, just use strerror */
-       StrNCpy(strerrbuf, strerror(errnum), buflen);
+       strlcpy(strerrbuf, strerror(errnum), buflen);
 
        return strerrbuf;
 #endif