Skip to content

Commit c92f7e2

Browse files
tglsfdctglsfdc
committed
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.
1 parent 996b203 commit c92f7e2

File tree

11 files changed

+72
-73
lines changed

11 files changed

+72
-73
lines changed

‎src/backend/commands/prepare.c

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* Copyright (c) 2002-2006, PostgreSQL Global Development Group
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.64 2006/09/07 22:52:00 tgl Exp $
13+
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.65 2006/09/27 18:40:09 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -314,18 +314,14 @@ StorePreparedStatement(const char *stmt_name,
314314
MemoryContext oldcxt,
315315
entrycxt;
316316
char *qstring;
317-
char key[NAMEDATALEN];
318317
bool found;
319318

320319
/* Initialize the hash table, if necessary */
321320
if (!prepared_queries)
322321
InitQueryHashTable();
323322

324323
/* Check for pre-existing entry of same name */
325-
/* See notes in FetchPreparedStatement */
326-
StrNCpy(key, stmt_name, sizeof(key));
327-
328-
hash_search(prepared_queries, key, HASH_FIND, &found);
324+
hash_search(prepared_queries, stmt_name, HASH_FIND, &found);
329325

330326
if (found)
331327
ereport(ERROR,
@@ -355,7 +351,7 @@ StorePreparedStatement(const char *stmt_name,
355351

356352
/* Now we can add entry to hash table */
357353
entry = (PreparedStatement *) hash_search(prepared_queries,
358-
key,
354+
stmt_name,
359355
HASH_ENTER,
360356
&found);
361357

@@ -384,27 +380,17 @@ StorePreparedStatement(const char *stmt_name,
384380
PreparedStatement *
385381
FetchPreparedStatement(const char *stmt_name, bool throwError)
386382
{
387-
char key[NAMEDATALEN];
388383
PreparedStatement *entry;
389384

390385
/*
391386
* If the hash table hasn't been initialized, it can't be storing
392387
* anything, therefore it couldn't possibly store our plan.
393388
*/
394389
if (prepared_queries)
395-
{
396-
/*
397-
* We can't just use the statement name as supplied by the user: the
398-
* hash package is picky enough that it needs to be NUL-padded out to
399-
* the appropriate length to work correctly.
400-
*/
401-
StrNCpy(key, stmt_name, sizeof(key));
402-
403390
entry = (PreparedStatement *) hash_search(prepared_queries,
404-
key,
391+
stmt_name,
405392
HASH_FIND,
406393
NULL);
407-
}
408394
else
409395
entry = NULL;
410396

‎src/backend/nodes/read.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/nodes/read.c,v 1.48 2006/03/05 15:58:28 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/nodes/read.c,v 1.49 2006/09/27 18:40:09 tgl Exp $
1313
*
1414
* HISTORY
1515
* AUTHOR DATE MAJOR EVENT
@@ -408,7 +408,7 @@ nodeRead(char *token, int tok_len)
408408
char *val = palloc(tok_len);
409409

410410
/* skip leading 'b' */
411-
strncpy(val, token + 1, tok_len - 1);
411+
memcpy(val, token + 1, tok_len - 1);
412412
val[tok_len - 1] = '\0';
413413
result = (Node *) makeBitString(val);
414414
break;

‎src/backend/storage/ipc/shmem.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.95 2006/08/01 19:03:11 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.96 2006/09/27 18:40:09 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -456,13 +456,9 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
456456
void *
457457
ShmemInitStruct(const char *name, Size size, bool *foundPtr)
458458
{
459-
ShmemIndexEnt *result,
460-
item;
459+
ShmemIndexEnt *result;
461460
void *structPtr;
462461

463-
strncpy(item.key, name, SHMEM_INDEX_KEYSIZE);
464-
item.location = BAD_LOCATION;
465-
466462
LWLockAcquire(ShmemIndexLock, LW_EXCLUSIVE);
467463

468464
if (!ShmemIndex)
@@ -498,7 +494,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
498494

499495
/* look it up in the shmem index */
500496
result = (ShmemIndexEnt *)
501-
hash_search(ShmemIndex, (void *) &item, HASH_ENTER_NULL, foundPtr);
497+
hash_search(ShmemIndex, name, HASH_ENTER_NULL, foundPtr);
502498

503499
if (!result)
504500
{
@@ -533,12 +529,13 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
533529
{
534530
/* out of memory */
535531
Assert(ShmemIndex);
536-
hash_search(ShmemIndex, (void *) &item, HASH_REMOVE, NULL);
532+
hash_search(ShmemIndex, name, HASH_REMOVE, NULL);
537533
LWLockRelease(ShmemIndexLock);
538534

539535
ereport(WARNING,
540536
(errcode(ERRCODE_OUT_OF_MEMORY),
541-
errmsg("could not allocate shared memory segment \"%s\"", name)));
537+
errmsg("could not allocate shared memory segment \"%s\"",
538+
name)));
542539
*foundPtr = FALSE;
543540
return NULL;
544541
}

‎src/backend/utils/error/elog.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*
4343
*
4444
* IDENTIFICATION
45-
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.173 2006/07/14 14:52:25 momjian Exp $
45+
* $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.174 2006/09/27 18:40:09 tgl Exp $
4646
*
4747
*-------------------------------------------------------------------------
4848
*/
@@ -1225,6 +1225,7 @@ write_syslog(int level, const char *line)
12251225
while (len > 0)
12261226
{
12271227
char buf[PG_SYSLOG_LIMIT + 1];
1228+
const char *nlpos;
12281229
int buflen;
12291230
int i;
12301231

@@ -1236,12 +1237,15 @@ write_syslog(int level, const char *line)
12361237
continue;
12371238
}
12381239

1239-
strncpy(buf, line, PG_SYSLOG_LIMIT);
1240-
buf[PG_SYSLOG_LIMIT] = '\0';
1241-
if (strchr(buf, '\n') != NULL)
1242-
*strchr(buf, '\n') = '\0';
1243-
1244-
buflen = strlen(buf);
1240+
/* copy one line, or as much as will fit, to buf */
1241+
nlpos = strchr(line, '\n');
1242+
if (nlpos != NULL)
1243+
buflen = nlpos - line;
1244+
else
1245+
buflen = len;
1246+
buflen = Min(buflen, PG_SYSLOG_LIMIT);
1247+
memcpy(buf, line, buflen);
1248+
buf[buflen] = '\0';
12451249

12461250
/* trim to multibyte letter boundary */
12471251
buflen = pg_mbcliplen(buf, buflen, buflen);

‎src/backend/utils/fmgr/dfmgr.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.89 2006/08/16 04:32:48 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.90 2006/09/27 18:40:09 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -594,7 +594,6 @@ find_rendezvous_variable(const char *varName)
594594
{
595595
static HTAB *rendezvousHash = NULL;
596596

597-
char key[NAMEDATALEN];
598597
rendezvousHashEntry *hentry;
599598
bool found;
600599

@@ -612,12 +611,9 @@ find_rendezvous_variable(const char *varName)
612611
HASH_ELEM);
613612
}
614613

615-
/* Turn the varName into a fixed-size string */
616-
StrNCpy(key, varName, sizeof(key));
617-
618614
/* Find or create the hashtable entry for this varName */
619615
hentry = (rendezvousHashEntry *) hash_search(rendezvousHash,
620-
key,
616+
varName,
621617
HASH_ENTER,
622618
&found);
623619

‎src/backend/utils/hash/dynahash.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
*
2828
* IDENTIFICATION
29-
* $PostgreSQL: pgsql/src/backend/utils/hash/dynahash.c,v 1.71 2006/08/14 12:39:55 tgl Exp $
29+
* $PostgreSQL: pgsql/src/backend/utils/hash/dynahash.c,v 1.72 2006/09/27 18:40:09 tgl Exp $
3030
*
3131
*-------------------------------------------------------------------------
3232
*/
@@ -209,6 +209,20 @@ DynaHashAlloc(Size size)
209209
}
210210

211211

212+
/*
213+
* HashCompareFunc for string keys
214+
*
215+
* Because we copy keys with strlcpy(), they will be truncated at keysize-1
216+
* bytes, so we can only compare that many ... hence strncmp is almost but
217+
* not quite the right thing.
218+
*/
219+
static int
220+
string_compare(const char *key1, const char *key2, Size keysize)
221+
{
222+
return strncmp(key1, key2, keysize - 1);
223+
}
224+
225+
212226
/************************** CREATE ROUTINES **********************/
213227

214228
/*
@@ -273,24 +287,24 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags)
273287
hashp->hash = string_hash; /* default hash function */
274288

275289
/*
276-
* If you don't specify a match function, it defaults to strncmp() if you
277-
* used string_hash (either explicitly or by default) and to memcmp()
278-
* otherwise. (Prior to PostgreSQL 7.4, memcmp() was always used.)
290+
* If you don't specify a match function, it defaults to string_compare
291+
* if you used string_hash (either explicitly or by default) and to memcmp
292+
* otherwise. (Prior to PostgreSQL 7.4, memcmp was always used.)
279293
*/
280294
if (flags & HASH_COMPARE)
281295
hashp->match = info->match;
282296
else if (hashp->hash == string_hash)
283-
hashp->match = (HashCompareFunc) strncmp;
297+
hashp->match = (HashCompareFunc) string_compare;
284298
else
285299
hashp->match = memcmp;
286300

287301
/*
288-
* Similarly, the key-copying function defaults to strncpy() or memcpy().
302+
* Similarly, the key-copying function defaults to strlcpy or memcpy.
289303
*/
290304
if (flags & HASH_KEYCOPY)
291305
hashp->keycopy = info->keycopy;
292306
else if (hashp->hash == string_hash)
293-
hashp->keycopy = (HashCopyFunc) strncpy;
307+
hashp->keycopy = (HashCopyFunc) strlcpy;
294308
else
295309
hashp->keycopy = memcpy;
296310

‎src/backend/utils/hash/hashfn.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/backend/utils/hash/hashfn.c,v 1.27 2006/07/14 14:52:25 momjian Exp $
12+
* $PostgreSQL: pgsql/src/backend/utils/hash/hashfn.c,v 1.28 2006/09/27 18:40:09 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -27,8 +27,16 @@
2727
uint32
2828
string_hash(const void *key, Size keysize)
2929
{
30+
/*
31+
* If the string exceeds keysize-1 bytes, we want to hash only that many,
32+
* because when it is copied into the hash table it will be truncated at
33+
* that length.
34+
*/
35+
Size s_len = strlen((const char *) key);
36+
37+
s_len = Min(s_len, keysize-1);
3038
return DatumGetUInt32(hash_any((const unsigned char *) key,
31-
(int) strlen((const char *) key)));
39+
(int) s_len));
3240
}
3341

3442
/*

‎src/backend/utils/misc/ps_status.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* to contain some useful information. Mechanism differs wildly across
66
* platforms.
77
*
8-
* $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.31 2006/06/27 22:16:44 momjian Exp $
8+
* $PostgreSQL: pgsql/src/backend/utils/misc/ps_status.c,v 1.32 2006/09/27 18:40:10 tgl Exp $
99
*
1010
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
1111
* various details abducted from various places
@@ -300,7 +300,7 @@ set_ps_display(const char *activity, bool force)
300300
#endif
301301

302302
/* Update ps_buffer to contain both fixed part and activity */
303-
StrNCpy(ps_buffer + ps_buffer_fixed_size, activity,
303+
strlcpy(ps_buffer + ps_buffer_fixed_size, activity,
304304
ps_buffer_size - ps_buffer_fixed_size);
305305

306306
/* Transmit new setting to kernel, if necessary */

‎src/backend/utils/mmgr/portalmem.c

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
1414
* IDENTIFICATION
15-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.94 2006/09/07 22:52:01 tgl Exp $
15+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.95 2006/09/27 18:40:10 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -54,12 +54,10 @@ static HTAB *PortalHashTable = NULL;
5454

5555
#define PortalHashTableLookup(NAME, PORTAL) \
5656
do { \
57-
PortalHashEnt *hentry; char key[MAX_PORTALNAME_LEN]; \
57+
PortalHashEnt *hentry; \
5858
\
59-
MemSet(key, 0, MAX_PORTALNAME_LEN); \
60-
StrNCpy(key, NAME, MAX_PORTALNAME_LEN); \
6159
hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
62-
key, HASH_FIND, NULL); \
60+
(NAME), HASH_FIND, NULL); \
6361
if (hentry) \
6462
PORTAL = hentry->portal; \
6563
else \
@@ -68,12 +66,10 @@ do { \
6866

6967
#define PortalHashTableInsert(PORTAL, NAME) \
7068
do { \
71-
PortalHashEnt *hentry; bool found; char key[MAX_PORTALNAME_LEN]; \
69+
PortalHashEnt *hentry; bool found; \
7270
\
73-
MemSet(key, 0, MAX_PORTALNAME_LEN); \
74-
StrNCpy(key, NAME, MAX_PORTALNAME_LEN); \
7571
hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
76-
key, HASH_ENTER, &found); \
72+
(NAME), HASH_ENTER, &found); \
7773
if (found) \
7874
elog(ERROR, "duplicate portal name"); \
7975
hentry->portal = PORTAL; \
@@ -83,12 +79,10 @@ do { \
8379

8480
#define PortalHashTableDelete(PORTAL) \
8581
do { \
86-
PortalHashEnt *hentry; char key[MAX_PORTALNAME_LEN]; \
82+
PortalHashEnt *hentry; \
8783
\
88-
MemSet(key, 0, MAX_PORTALNAME_LEN); \
89-
StrNCpy(key, PORTAL->name, MAX_PORTALNAME_LEN); \
9084
hentry = (PortalHashEnt *) hash_search(PortalHashTable, \
91-
key, HASH_REMOVE, NULL); \
85+
PORTAL->name, HASH_REMOVE, NULL); \
9286
if (hentry == NULL) \
9387
elog(WARNING, "trying to delete portal name that does not exist"); \
9488
} while(0)

0 commit comments

Comments
 (0)