Commit c92f7e25 authored by Tom Lane's avatar Tom Lane

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.
parent 996b203e
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
* Copyright (c) 2002-2006, PostgreSQL Global Development Group * Copyright (c) 2002-2006, PostgreSQL Global Development Group
* *
* IDENTIFICATION * 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, ...@@ -314,7 +314,6 @@ StorePreparedStatement(const char *stmt_name,
MemoryContext oldcxt, MemoryContext oldcxt,
entrycxt; entrycxt;
char *qstring; char *qstring;
char key[NAMEDATALEN];
bool found; bool found;
/* Initialize the hash table, if necessary */ /* Initialize the hash table, if necessary */
...@@ -322,10 +321,7 @@ StorePreparedStatement(const char *stmt_name, ...@@ -322,10 +321,7 @@ StorePreparedStatement(const char *stmt_name,
InitQueryHashTable(); InitQueryHashTable();
/* Check for pre-existing entry of same name */ /* Check for pre-existing entry of same name */
/* See notes in FetchPreparedStatement */ hash_search(prepared_queries, stmt_name, HASH_FIND, &found);
StrNCpy(key, stmt_name, sizeof(key));
hash_search(prepared_queries, key, HASH_FIND, &found);
if (found) if (found)
ereport(ERROR, ereport(ERROR,
...@@ -355,7 +351,7 @@ StorePreparedStatement(const char *stmt_name, ...@@ -355,7 +351,7 @@ StorePreparedStatement(const char *stmt_name,
/* Now we can add entry to hash table */ /* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries, entry = (PreparedStatement *) hash_search(prepared_queries,
key, stmt_name,
HASH_ENTER, HASH_ENTER,
&found); &found);
...@@ -384,7 +380,6 @@ StorePreparedStatement(const char *stmt_name, ...@@ -384,7 +380,6 @@ StorePreparedStatement(const char *stmt_name,
PreparedStatement * PreparedStatement *
FetchPreparedStatement(const char *stmt_name, bool throwError) FetchPreparedStatement(const char *stmt_name, bool throwError)
{ {
char key[NAMEDATALEN];
PreparedStatement *entry; PreparedStatement *entry;
/* /*
...@@ -392,19 +387,10 @@ FetchPreparedStatement(const char *stmt_name, bool throwError) ...@@ -392,19 +387,10 @@ FetchPreparedStatement(const char *stmt_name, bool throwError)
* anything, therefore it couldn't possibly store our plan. * anything, therefore it couldn't possibly store our plan.
*/ */
if (prepared_queries) 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, entry = (PreparedStatement *) hash_search(prepared_queries,
key, stmt_name,
HASH_FIND, HASH_FIND,
NULL); NULL);
}
else else
entry = NULL; entry = NULL;
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
* *
* *
* IDENTIFICATION * 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 * HISTORY
* AUTHOR DATE MAJOR EVENT * AUTHOR DATE MAJOR EVENT
...@@ -408,7 +408,7 @@ nodeRead(char *token, int tok_len) ...@@ -408,7 +408,7 @@ nodeRead(char *token, int tok_len)
char *val = palloc(tok_len); char *val = palloc(tok_len);
/* skip leading 'b' */ /* skip leading 'b' */
strncpy(val, token + 1, tok_len - 1); memcpy(val, token + 1, tok_len - 1);
val[tok_len - 1] = '\0'; val[tok_len - 1] = '\0';
result = (Node *) makeBitString(val); result = (Node *) makeBitString(val);
break; break;
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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 */ ...@@ -456,13 +456,9 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
void * void *
ShmemInitStruct(const char *name, Size size, bool *foundPtr) ShmemInitStruct(const char *name, Size size, bool *foundPtr)
{ {
ShmemIndexEnt *result, ShmemIndexEnt *result;
item;
void *structPtr; void *structPtr;
strncpy(item.key, name, SHMEM_INDEX_KEYSIZE);
item.location = BAD_LOCATION;
LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE); LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
if (!ShmemIndex) if (!ShmemIndex)
...@@ -498,7 +494,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) ...@@ -498,7 +494,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
/* look it up in the shmem index */ /* look it up in the shmem index */
result = (ShmemIndexEnt *) result = (ShmemIndexEnt *)
hash_search(ShmemIndex, (void *) &item, HASH_ENTER_NULL, foundPtr); hash_search(ShmemIndex, name, HASH_ENTER_NULL, foundPtr);
if (!result) if (!result)
{ {
...@@ -533,12 +529,13 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) ...@@ -533,12 +529,13 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
{ {
/* out of memory */ /* out of memory */
Assert(ShmemIndex); Assert(ShmemIndex);
hash_search(ShmemIndex, (void *) &item, HASH_REMOVE, NULL); hash_search(ShmemIndex, name, HASH_REMOVE, NULL);
LWLockRelease(ShmemIndexLock); LWLockRelease(ShmemIndexLock);
ereport(WARNING, ereport(WARNING,
(errcode(ERRCODE_OUT_OF_MEMORY), (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; *foundPtr = FALSE;
return NULL; return NULL;
} }
......
...@@ -42,7 +42,7 @@ ...@@ -42,7 +42,7 @@
* *
* *
* IDENTIFICATION * 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) ...@@ -1225,6 +1225,7 @@ write_syslog(int level, const char *line)
while (len > 0) while (len > 0)
{ {
char buf[PG_SYSLOG_LIMIT + 1]; char buf[PG_SYSLOG_LIMIT + 1];
const char *nlpos;
int buflen; int buflen;
int i; int i;
...@@ -1236,12 +1237,15 @@ write_syslog(int level, const char *line) ...@@ -1236,12 +1237,15 @@ write_syslog(int level, const char *line)
continue; continue;
} }
strncpy(buf, line, PG_SYSLOG_LIMIT); /* copy one line, or as much as will fit, to buf */
buf[PG_SYSLOG_LIMIT] = '\0'; nlpos = strchr(line, '\n');
if (strchr(buf, '\n') != NULL) if (nlpos != NULL)
*strchr(buf, '\n') = '\0'; buflen = nlpos - line;
else
buflen = strlen(buf); buflen = len;
buflen = Min(buflen, PG_SYSLOG_LIMIT);
memcpy(buf, line, buflen);
buf[buflen] = '\0';
/* trim to multibyte letter boundary */ /* trim to multibyte letter boundary */
buflen = pg_mbcliplen(buf, buflen, buflen); buflen = pg_mbcliplen(buf, buflen, buflen);
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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) ...@@ -594,7 +594,6 @@ find_rendezvous_variable(const char *varName)
{ {
static HTAB *rendezvousHash = NULL; static HTAB *rendezvousHash = NULL;
char key[NAMEDATALEN];
rendezvousHashEntry *hentry; rendezvousHashEntry *hentry;
bool found; bool found;
...@@ -612,12 +611,9 @@ find_rendezvous_variable(const char *varName) ...@@ -612,12 +611,9 @@ find_rendezvous_variable(const char *varName)
HASH_ELEM); HASH_ELEM);
} }
/* Turn the varName into a fixed-size string */
StrNCpy(key, varName, sizeof(key));
/* Find or create the hashtable entry for this varName */ /* Find or create the hashtable entry for this varName */
hentry = (rendezvousHashEntry *) hash_search(rendezvousHash, hentry = (rendezvousHashEntry *) hash_search(rendezvousHash,
key, varName,
HASH_ENTER, HASH_ENTER,
&found); &found);
......
...@@ -26,7 +26,7 @@ ...@@ -26,7 +26,7 @@
* *
* *
* IDENTIFICATION * 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) ...@@ -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 **********************/ /************************** CREATE ROUTINES **********************/
/* /*
...@@ -273,24 +287,24 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags) ...@@ -273,24 +287,24 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
hashp->hash = string_hash; /* default hash function */ hashp->hash = string_hash; /* default hash function */
/* /*
* If you don't specify a match function, it defaults to strncmp() if you * If you don't specify a match function, it defaults to string_compare
* used string_hash (either explicitly or by default) and to memcmp() * if you used string_hash (either explicitly or by default) and to memcmp
* otherwise. (Prior to PostgreSQL 7.4, memcmp() was always used.) * otherwise. (Prior to PostgreSQL 7.4, memcmp was always used.)
*/ */
if (flags & HASH_COMPARE) if (flags & HASH_COMPARE)
hashp->match = info->match; hashp->match = info->match;
else if (hashp->hash == string_hash) else if (hashp->hash == string_hash)
hashp->match = (HashCompareFunc) strncmp; hashp->match = (HashCompareFunc) string_compare;
else else
hashp->match = memcmp; 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) if (flags & HASH_KEYCOPY)
hashp->keycopy = info->keycopy; hashp->keycopy = info->keycopy;
else if (hashp->hash == string_hash) else if (hashp->hash == string_hash)
hashp->keycopy = (HashCopyFunc) strncpy; hashp->keycopy = (HashCopyFunc) strlcpy;
else else
hashp->keycopy = memcpy; hashp->keycopy = memcpy;
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
* *
* *
* IDENTIFICATION * 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 $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -27,8 +27,16 @@ ...@@ -27,8 +27,16 @@
uint32 uint32
string_hash(const void *key, Size keysize) 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, return DatumGetUInt32(hash_any((const unsigned char *) key,
(int) strlen((const char *) key))); (int) s_len));
} }
/* /*
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
* to contain some useful information. Mechanism differs wildly across * to contain some useful information. Mechanism differs wildly across
* platforms. * 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 * Copyright (c) 2000-2006, PostgreSQL Global Development Group
* various details abducted from various places * various details abducted from various places
...@@ -300,7 +300,7 @@ set_ps_display(const char *activity, bool force) ...@@ -300,7 +300,7 @@ set_ps_display(const char *activity, bool force)
#endif #endif
/* Update ps_buffer to contain both fixed part and activity */ /* 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); ps_buffer_size - ps_buffer_fixed_size);
/* Transmit new setting to kernel, if necessary */ /* Transmit new setting to kernel, if necessary */
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* IDENTIFICATION * 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; ...@@ -54,12 +54,10 @@ static HTAB *PortalHashTable = NULL;
#define PortalHashTableLookup(NAME, PORTAL) \ #define PortalHashTableLookup(NAME, PORTAL) \
do { \ 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, \ hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
key, HASH_FIND, NULL); \ (NAME), HASH_FIND, NULL); \
if (hentry) \ if (hentry) \
PORTAL = hentry->portal; \ PORTAL = hentry->portal; \
else \ else \
...@@ -68,12 +66,10 @@ do { \ ...@@ -68,12 +66,10 @@ do { \
#define PortalHashTableInsert(PORTAL, NAME) \ #define PortalHashTableInsert(PORTAL, NAME) \
do { \ 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, \ hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
key, HASH_ENTER, &found); \ (NAME), HASH_ENTER, &found); \
if (found) \ if (found) \
elog(ERROR, "duplicate portal name"); \ elog(ERROR, "duplicate portal name"); \
hentry->portal = PORTAL; \ hentry->portal = PORTAL; \
...@@ -83,12 +79,10 @@ do { \ ...@@ -83,12 +79,10 @@ do { \
#define PortalHashTableDelete(PORTAL) \ #define PortalHashTableDelete(PORTAL) \
do { \ 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, \ hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
key, HASH_REMOVE, NULL); \ PORTAL->name, HASH_REMOVE, NULL); \
if (hentry == NULL) \ if (hentry == NULL) \
elog(WARNING, "trying to delete portal name that does not exist"); \ elog(WARNING, "trying to delete portal name that does not exist"); \
} while(0) } while(0)
......
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * 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, ...@@ -174,7 +174,7 @@ join_path_components(char *ret_path,
const char *head, const char *tail) const char *head, const char *tail)
{ {
if (ret_path != head) if (ret_path != head)
StrNCpy(ret_path, head, MAXPGPATH); strlcpy(ret_path, head, MAXPGPATH);
/* /*
* Remove any leading "." and ".." in the tail component, adjusting head * Remove any leading "." and ".." in the tail component, adjusting head
...@@ -493,7 +493,7 @@ make_relative_path(char *ret_path, const char *target_path, ...@@ -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 * Set up my_exec_path without the actual executable name, and
* canonicalize to simplify comparison to bin_path. * 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 */ trim_directory(ret_path); /* remove my executable name */
canonicalize_path(ret_path); canonicalize_path(ret_path);
...@@ -513,7 +513,7 @@ make_relative_path(char *ret_path, const char *target_path, ...@@ -513,7 +513,7 @@ make_relative_path(char *ret_path, const char *target_path,
} }
no_match: no_match:
StrNCpy(ret_path, target_path, MAXPGPATH); strlcpy(ret_path, target_path, MAXPGPATH);
canonicalize_path(ret_path); canonicalize_path(ret_path);
} }
...@@ -625,7 +625,7 @@ get_home_path(char *ret_path) ...@@ -625,7 +625,7 @@ get_home_path(char *ret_path)
if (pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) != 0) if (pqGetpwuid(geteuid(), &pwdstr, pwdbuf, sizeof(pwdbuf), &pwd) != 0)
return false; return false;
StrNCpy(ret_path, pwd->pw_dir, MAXPGPATH); strlcpy(ret_path, pwd->pw_dir, MAXPGPATH);
return true; return true;
#else #else
char tmppath[MAX_PATH]; char tmppath[MAX_PATH];
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
* *
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * 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) ...@@ -81,7 +81,7 @@ pqStrerror(int errnum, char *strerrbuf, size_t buflen)
#endif #endif
#else #else
/* no strerror_r() available, just use strerror */ /* no strerror_r() available, just use strerror */
StrNCpy(strerrbuf, strerror(errnum), buflen); strlcpy(strerrbuf, strerror(errnum), buflen);
return strerrbuf; return strerrbuf;
#endif #endif
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment