Replace strncpy with strlcpy in selected places that seem possibly relevant
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 27 Sep 2006 18:40:10 +0000 (18:40 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 27 Sep 2006 18:40:10 +0000 (18:40 +0000)
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.

src/backend/commands/prepare.c
src/backend/nodes/read.c
src/backend/storage/ipc/shmem.c
src/backend/utils/error/elog.c
src/backend/utils/fmgr/dfmgr.c
src/backend/utils/hash/dynahash.c
src/backend/utils/hash/hashfn.c
src/backend/utils/misc/ps_status.c
src/backend/utils/mmgr/portalmem.c
src/port/path.c
src/port/thread.c

index eccce9d10d6492236bdbb468268ffa5354387d7b..46824a48e5dc8bf8849774a3d4541c09dbbf2f22 100644 (file)
@@ -10,7 +10,7 @@
  * Copyright (c) 2002-2006, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.64 2006/09/07 22:52:00 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.65 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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;
 
index 14ccb88942ce1975ae750dcd425609fa7bedf08a..4563478899286e00da731effb530d37126936b22 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/nodes/read.c,v 1.48 2006/03/05 15:58:28 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/nodes/read.c,v 1.49 2006/09/27 18:40:09 tgl Exp $
  *
  * HISTORY
  *       AUTHOR                        DATE                    MAJOR EVENT
@@ -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;
index 3c48173f2f69c835179ccd2527eaa0315d9efeac..4cff4c19f336d96adb2f5ce3b8854c484ce4c7a9 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.95 2006/08/01 19:03:11 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.96 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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;
                }
index e46d16fa55d6f908a1785bdaeabf17e44ff1a691..2aff7f790aa1520e3bd8f2f4f8ee3c94d3074825 100644 (file)
@@ -42,7 +42,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.173 2006/07/14 14:52:25 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.174 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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);
index b95180d17efe7c18df912abf786d55dc14a2d09f..b12674026e2d7be8956044b5fd95bfd6066a1449 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.89 2006/08/16 04:32:48 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.90 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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);
 
index b2974da999ab64ff2d9f8368b5f724dbe91bdeb3..aa0a6b11f2ac2992bf5608a8dcca052e6612b78b 100644 (file)
@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/hash/dynahash.c,v 1.71 2006/08/14 12:39:55 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/hash/dynahash.c,v 1.72 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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;
 
index ed0826e8e51a3c0590ea7c3f8c3027f1c8d0a497..af528881eff36c5bea59f87a38d07a855b7627c0 100644 (file)
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/hash/hashfn.c,v 1.27 2006/07/14 14:52:25 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/hash/hashfn.c,v 1.28 2006/09/27 18:40:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 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));
 }
 
 /*
index d23aa563f79b4f5d15783315ac2fe13a1a494a41..f6c4c588b203bb23d46245334c29d709f9f290ae 100644 (file)
@@ -5,7 +5,7 @@
  * to contain some useful information. Mechanism differs wildly across
  * platforms.
  *
- * $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.31 2006/06/27 22:16:44 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.32 2006/09/27 18:40:10 tgl Exp $
  *
  * Copyright (c) 2000-2006, PostgreSQL Global Development Group
  * various details abducted from various places
@@ -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 */
index e29ffb53c98a487abccfd41ddc9b4c3e589d311b..883f075eec227c94f23ad2a366b5a62a449c7805 100644 (file)
@@ -12,7 +12,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.94 2006/09/07 22:52:01 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.95 2006/09/27 18:40:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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)
index 4acb8046bfcb8991292461c576662defb132915b..a7d5ec7c4528b94e6a149f93fb14ef6ae6f73ae7 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/port/path.c,v 1.68 2006/09/22 21:39:58 tgl Exp $
+ *       $PostgreSQL: pgsql/src/port/path.c,v 1.69 2006/09/27 18:40:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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];
index 04f42f55b2c717e866a339adc39d77166d0f4f7a..74e6508c36ada39b5f2a216265d696ef3d608aa3 100644 (file)
@@ -7,7 +7,7 @@
  *
  * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/port/thread.c,v 1.34 2006/07/06 02:12:32 momjian Exp $
+ * $PostgreSQL: pgsql/src/port/thread.c,v 1.35 2006/09/27 18:40:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -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