git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff

Replace remaining StrNCpy() by strlcpy()

author Peter Eisentraut <peter@eisentraut.org>

2020年8月10日 16:51:31 +0000 (18:51 +0200)

committer Peter Eisentraut <peter@eisentraut.org>

2020年8月10日 21:20:37 +0000 (23:20 +0200)

They are equivalent, except that StrNCpy() zero-fills the entire
destination buffer instead of providing just one trailing zero. For
all but a tiny number of callers, that's just overhead rather than
being desirable.

Remove StrNCpy() as it is now unused.

In some cases, namestrcpy() is the more appropriate function to use.
While we're here, simplify the API of namestrcpy(): Remove the return
value, don't check for NULL input. Nothing was using that anyway.
Also, remove a few unused name-related functions.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/44f5e198-36f6-6cdb-7fa9-60e34784daae%402ndquadrant.com

20 files changed:

contrib/pgcrypto/crypt-des.c patch | blob | blame | history

src/backend/access/transam/slru.c patch | blob | blame | history

src/backend/access/transam/xlogarchive.c patch | blob | blame | history

src/backend/catalog/pg_constraint.c patch | blob | blame | history

src/backend/commands/indexcmds.c patch | blob | blame | history

src/backend/commands/statscmds.c patch | blob | blame | history

src/backend/commands/tablecmds.c patch | blob | blame | history

src/backend/postmaster/pgstat.c patch | blob | blame | history

src/backend/replication/logical/logical.c patch | blob | blame | history

src/backend/replication/slot.c patch | blob | blame | history

src/backend/utils/adt/formatting.c patch | blob | blame | history

src/backend/utils/adt/name.c patch | blob | blame | history

src/backend/utils/adt/pg_locale.c patch | blob | blame | history

src/backend/utils/adt/ruleutils.c patch | blob | blame | history

src/common/exec.c patch | blob | blame | history

src/include/c.h patch | blob | blame | history

src/include/utils/builtins.h patch | blob | blame | history

src/interfaces/ecpg/pgtypeslib/dt_common.c patch | blob | blame | history

src/interfaces/ecpg/test/pg_regress_ecpg.c patch | blob | blame | history

src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c patch | blob | blame | history

diff --git a/contrib/pgcrypto/crypt-des.c b/contrib/pgcrypto/crypt-des.c

index 6efaa609c9d18070b5a33d47c8b3e4e867704279..98c30ea122e3bce5419127504045ecf12bdee061 100644 (file)

--- a/contrib/pgcrypto/crypt-des.c

+++ b/contrib/pgcrypto/crypt-des.c

@@ -720,7 +720,7 @@ px_crypt_des(const char *key, const char *setting)

if (des_setkey((char *) keybuf))

return NULL;

}

- StrNCpy(output, setting, 10);

+ strlcpy(output, setting, 10);

* Double check that we weren't given a short setting. If we were, the

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c

index 9e145f1c36acb245a1aa9791a970ba0a4bd206dd..d1dbb43e096c1e07f5488e1675525be87f0c14a0 100644 (file)

--- a/src/backend/access/transam/slru.c

+++ b/src/backend/access/transam/slru.c

@@ -252,7 +252,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,

ctl->shared = shared;

ctl->do_fsync = true; /* default behavior */

- StrNCpy(ctl->Dir, subdir, sizeof(ctl->Dir));

+ strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));

}

diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c

index cdd586fcfbae5a328c1d435525e2f011a687750b..8f8734dc1d4ec1e3744a5445fedb168adaa805e4 100644 (file)

--- a/src/backend/access/transam/xlogarchive.c

+++ b/src/backend/access/transam/xlogarchive.c

@@ -323,7 +323,7 @@ ExecuteRecoveryCommand(const char *command, const char *commandName, bool failOn

case 'r':

/* %r: filename of last restartpoint */

sp++;

- StrNCpy(dp, lastRestartPointFname, endp - dp);

+ strlcpy(dp, lastRestartPointFname, endp - dp);

dp += strlen(dp);

break;

case '%':

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c

index fdc63e7dea16b3ab41b29af4837b0270a84b1d7c..6a6b2cb8c0c8449e39f28e6683e12b949b54849a 100644 (file)

--- a/src/backend/catalog/pg_constraint.c

+++ b/src/backend/catalog/pg_constraint.c

@@ -484,7 +484,7 @@ ChooseConstraintName(const char *name1, const char *name2,

conDesc = table_open(ConstraintRelationId, AccessShareLock);

/* try the unmodified label first */

- StrNCpy(modlabel, label, sizeof(modlabel));

+ strlcpy(modlabel, label, sizeof(modlabel));

for (;;)

{

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c

index 2baca12c5f475aad6fbf58df29bdfa36a4f20ee0..7819266a63063563c1c466446794647f7a2fc904 100644 (file)

--- a/src/backend/commands/indexcmds.c

+++ b/src/backend/commands/indexcmds.c

@@ -2246,7 +2246,7 @@ ChooseRelationName(const char *name1, const char *name2,

char modlabel[NAMEDATALEN];

/* try the unmodified label first */

- StrNCpy(modlabel, label, sizeof(modlabel));

+ strlcpy(modlabel, label, sizeof(modlabel));

for (;;)

{

diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c

index 974828545ca957f2294cffd5a15e9f1423690b2a..3057d89d50c01b53dabe8d9e6558ce5b2538d19e 100644 (file)

--- a/src/backend/commands/statscmds.c

+++ b/src/backend/commands/statscmds.c

@@ -681,7 +681,7 @@ ChooseExtendedStatisticName(const char *name1, const char *name2,

char modlabel[NAMEDATALEN];

/* try the unmodified label first */

- StrNCpy(modlabel, label, sizeof(modlabel));

+ strlcpy(modlabel, label, sizeof(modlabel));

for (;;)

{

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c

index ac53f79ada2a6884a714050289c7c0143cb8bdf7..cd989c95e517433b1a5edf4f3670618eac1dbc5a 100644 (file)

--- a/src/backend/commands/tablecmds.c

+++ b/src/backend/commands/tablecmds.c

@@ -606,7 +606,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,

* Truncate relname to appropriate length (probably a waste of time, as

* parser should have done this already).

- StrNCpy(relname, stmt->relation->relname, NAMEDATALEN);

+ strlcpy(relname, stmt->relation->relname, NAMEDATALEN);

* Check consistency of arguments

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c

index 15f92b66c6ba42f59db5f56383e06d7aa11da78b..73ce944fb1ce9e30b25581263efa86a27d7cdd36 100644 (file)

--- a/src/backend/postmaster/pgstat.c

+++ b/src/backend/postmaster/pgstat.c

@@ -4367,7 +4367,7 @@ pgstat_send_archiver(const char *xlog, bool failed)

pgstat_setheader(&msg.m_hdr, PGSTAT_MTYPE_ARCHIVER);

msg.m_failed = failed;

- StrNCpy(msg.m_xlog, xlog, sizeof(msg.m_xlog));

+ strlcpy(msg.m_xlog, xlog, sizeof(msg.m_xlog));

msg.m_timestamp = GetCurrentTimestamp();

pgstat_send(&msg, sizeof(msg));

}

diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c

index f5eb6bc3aff22467c82400adac0b49ffe48280c8..57c5b513ccf848a7a8ccb8ae4cb640edbc213035 100644 (file)

--- a/src/backend/replication/logical/logical.c

+++ b/src/backend/replication/logical/logical.c

@@ -39,6 +39,7 @@

#include "replication/snapbuild.h"

#include "storage/proc.h"

#include "storage/procarray.h"

+#include "utils/builtins.h"

#include "utils/memutils.h"

/* data for errcontext callback */

@@ -288,6 +289,7 @@ CreateInitDecodingContext(const char *plugin,

{

TransactionId xmin_horizon = InvalidTransactionId;

ReplicationSlot *slot;

+ NameData plugin_name;

LogicalDecodingContext *ctx;

MemoryContext old_context;

@@ -319,9 +321,14 @@ CreateInitDecodingContext(const char *plugin,

(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),

errmsg("cannot create logical replication slot in transaction that has performed writes")));

- /* register output plugin name with slot */

+ /*

+ * Register output plugin name with slot. We need the mutex to avoid

+ * concurrent reading of a partially copied string. But we don't want any

+ * complicated code while holding a spinlock, so do namestrcpy() outside.

+ */

+ namestrcpy(&plugin_name, plugin);

SpinLockAcquire(&slot->mutex);

- StrNCpy(NameStr(slot->data.plugin), plugin, NAMEDATALEN);

+ slot->data.plugin = plugin_name;

SpinLockRelease(&slot->mutex);

if (XLogRecPtrIsInvalid(restart_lsn))

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c

index 57bbb6288c6827f9a8f84b00426a31c56e3c511f..3dc01b6df22a966ccbab2ef9a5d144d48c86d7d7 100644 (file)

--- a/src/backend/replication/slot.c

+++ b/src/backend/replication/slot.c

@@ -275,7 +275,7 @@ ReplicationSlotCreate(const char *name, bool db_specific,

/* first initialize persistent data */

memset(&slot->data, 0, sizeof(ReplicationSlotPersistentData));

- StrNCpy(NameStr(slot->data.name), name, NAMEDATALEN);

+ namestrcpy(&slot->data.name, name);

slot->data.database = db_specific ? MyDatabaseId : InvalidOid;

slot->data.persistency = persistency;

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c

index 662643813660d3075272a8d0dc00689206b506f5..9de63686ecb599d84218140981991109238c984a 100644 (file)

--- a/src/backend/utils/adt/formatting.c

+++ b/src/backend/utils/adt/formatting.c

@@ -3890,7 +3890,7 @@ DCH_cache_getnew(const char *str, bool std)

elog(DEBUG_elog_output, "OLD: '%s' AGE: %d", old->str, old->age);

#endif

old->valid = false;

- StrNCpy(old->str, str, DCH_CACHE_SIZE + 1);

+ strlcpy(old->str, str, DCH_CACHE_SIZE + 1);

old->age = (++DCHCounter);

/* caller is expected to fill format, then set valid */

return old;

@@ -3904,7 +3904,7 @@ DCH_cache_getnew(const char *str, bool std)

DCHCache[n_DCHCache] = ent = (DCHCacheEntry *)

MemoryContextAllocZero(TopMemoryContext, sizeof(DCHCacheEntry));

ent->valid = false;

- StrNCpy(ent->str, str, DCH_CACHE_SIZE + 1);

+ strlcpy(ent->str, str, DCH_CACHE_SIZE + 1);

ent->std = std;

ent->age = (++DCHCounter);

/* caller is expected to fill format, then set valid */

@@ -4799,7 +4799,7 @@ NUM_cache_getnew(const char *str)

elog(DEBUG_elog_output, "OLD: \"%s\" AGE: %d", old->str, old->age);

#endif

old->valid = false;

- StrNCpy(old->str, str, NUM_CACHE_SIZE + 1);

+ strlcpy(old->str, str, NUM_CACHE_SIZE + 1);

old->age = (++NUMCounter);

/* caller is expected to fill format and Num, then set valid */

return old;

@@ -4813,7 +4813,7 @@ NUM_cache_getnew(const char *str)

NUMCache[n_NUMCache] = ent = (NUMCacheEntry *)

MemoryContextAllocZero(TopMemoryContext, sizeof(NUMCacheEntry));

ent->valid = false;

- StrNCpy(ent->str, str, NUM_CACHE_SIZE + 1);

+ strlcpy(ent->str, str, NUM_CACHE_SIZE + 1);

ent->age = (++NUMCounter);

/* caller is expected to fill format and Num, then set valid */

++n_NUMCache;

diff --git a/src/backend/utils/adt/name.c b/src/backend/utils/adt/name.c

index 64877f67e0103445ae57be43514ab8eb4f504584..a3ce3f3d1e182deb0a6b58e960afd52b8f4edc61 100644 (file)

--- a/src/backend/utils/adt/name.c

+++ b/src/backend/utils/adt/name.c

@@ -229,53 +229,13 @@ btnamesortsupport(PG_FUNCTION_ARGS)

* MISCELLANEOUS PUBLIC ROUTINES *

*****************************************************************************/

-int

-namecpy(Name n1, const NameData *n2)

- if (!n1 || !n2)

- return -1;

- StrNCpy(NameStr(*n1), NameStr(*n2), NAMEDATALEN);

- return 0;

-#ifdef NOT_USED

-int

-namecat(Name n1, Name n2)

- return namestrcat(n1, NameStr(*n2)); /* n2 can't be any longer than n1 */

-#endif

-int

+void

namestrcpy(Name name, const char *str)

{

- if (!name || !str)

- return -1;

- StrNCpy(NameStr(*name), str, NAMEDATALEN);

- return 0;

-#ifdef NOT_USED

-int

-namestrcat(Name name, const char *str)

- int i;

- char *p,

- *q;

- if (!name || !str)

- return -1;

- for (i = 0, p = NameStr(*name); i < NAMEDATALEN && *p; ++i, ++p)

- ;

- for (q = str; i < NAMEDATALEN; ++i, ++p, ++q)

- {

- *p = *q;

- if (!*q)

- break;

- }

- return 0;

+ /* NB: We need to zero-pad the destination. */

+ strncpy(NameStr(*name), str, NAMEDATALEN);

+ NameStr(*name)[NAMEDATALEN-1] = '0円';

}

-#endif

* Compare a NAME to a C string

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c

index 11d05c73acccc80c73d382e90042cace01caf70a..07299dbc09111b899f5d99adf042347334dd606b 100644 (file)

--- a/src/backend/utils/adt/pg_locale.c

+++ b/src/backend/utils/adt/pg_locale.c

@@ -75,16 +75,7 @@

#endif

#ifdef WIN32

-/*

- * This Windows file defines StrNCpy. We don't need it here, so we undefine

- * it to keep the compiler quiet, and undefine it again after the file is

- * included, so we don't accidentally use theirs.

- */

-#undef StrNCpy

#include <shlwapi.h>

-#ifdef StrNCpy

-#undef StrNCpy

-#endif

#endif

#define MAX_L10N_DATA 80

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c

index 2cbcb4b85e3b2b2d5a8a87db5207892357b2ce0e..60dd80c23c87d04f33f21718d4e456fecb456057 100644 (file)

--- a/src/backend/utils/adt/ruleutils.c

+++ b/src/backend/utils/adt/ruleutils.c

@@ -2489,7 +2489,7 @@ pg_get_userbyid(PG_FUNCTION_ARGS)

if (HeapTupleIsValid(roletup))

{

role_rec = (Form_pg_authid) GETSTRUCT(roletup);

- StrNCpy(NameStr(*result), NameStr(role_rec->rolname), NAMEDATALEN);

+ *result = role_rec->rolname;

ReleaseSysCache(roletup);

}

else

diff --git a/src/common/exec.c b/src/common/exec.c

index f39b0a294bf5fbf4af48d6413c8222276b37e53e..78bb486f999a2ca248a04715314ad784481434dd 100644 (file)

--- a/src/common/exec.c

+++ b/src/common/exec.c

@@ -144,7 +144,7 @@ find_my_exec(const char *argv0, char *retpath)

if (first_dir_separator(argv0) != NULL)

{

if (is_absolute_path(argv0))

- StrNCpy(retpath, argv0, MAXPGPATH);

+ strlcpy(retpath, argv0, MAXPGPATH);

else

join_path_components(retpath, cwd, argv0);

canonicalize_path(retpath);

@@ -184,7 +184,7 @@ find_my_exec(const char *argv0, char *retpath)

if (!endp)

endp = startp + strlen(startp); /* point to end */

- StrNCpy(test_path, startp, Min(endp - startp + 1, MAXPGPATH));

+ strlcpy(test_path, startp, Min(endp - startp + 1, MAXPGPATH));

if (is_absolute_path(test_path))

join_path_components(retpath, test_path, argv0);

diff --git a/src/include/c.h b/src/include/c.h

index f242e32edbe707f473746c0da8e46027a9f9144e..2c61ca8aa894dbc2fd385b6a0488cc00e0bed14b 100644 (file)

--- a/src/include/c.h

+++ b/src/include/c.h

@@ -932,35 +932,6 @@ extern void ExceptionalCondition(const char *conditionName,

#define Abs(x) ((x) >= 0 ? (x) : -(x))

-/*

- * StrNCpy

- * Like standard library function strncpy(), except that result string

- * is guaranteed to be null-terminated --- that is, at most N-1 bytes

- * of the source string will be kept.

- * Also, the macro returns no result (too hard to do that without

- * evaluating the arguments multiple times, which seems worse).

- *

- * BTW: when you need to copy a non-null-terminated string (like a text

- * datum) and add a null, do not do it with StrNCpy(..., len+1). That

- * might seem to work, but it fetches one byte more than there is in the

- * text object. One fine day you'll have a SIGSEGV because there isn't

- * another byte before the end of memory. Don't laugh, we've had real

- * live bug reports from real live users over exactly this mistake.

- * Do it honestly with "memcpy(dst,src,len); dst[len] = '0円';", instead.

- */

-#define StrNCpy(dst,src,len) \

- do \

- { \

- char * _dst = (dst); \

- Size _len = (len); \

- if (_len > 0) \

- { \

- strncpy(_dst, (src), _len); \

- _dst[_len-1] = '0円'; \

- } \

- } while (0)

/* Get a bit mask of the bits set in non-long aligned addresses */

#define LONG_ALIGN_MASK (sizeof(long) - 1)

diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h

index 3ca5e938f8f82d759d64844134e6d415c66107df..4db5ad3f12e5e2889169dd4cdbffa397ddd8413e 100644 (file)

--- a/src/include/utils/builtins.h

+++ b/src/include/utils/builtins.h

@@ -39,8 +39,7 @@ extern uint64 hex_decode(const char *src, size_t len, char *dst);

extern int2vector *buildint2vector(const int16 *int2s, int n);

/* name.c */

-extern int namecpy(Name n1, const NameData *n2);

-extern int namestrcpy(Name name, const char *str);

+extern void namestrcpy(Name name, const char *str);

extern int namestrcmp(Name name, const char *str);

/* numutils.c */

diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c

index 14cdf2d428b5d1eec1eef9666bc6b2a6369701d3..e8a8a0f0ed3edaaa72e634a91c8e804c2914bc27 100644 (file)

--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c

+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c

@@ -1015,7 +1015,7 @@ abstime2tm(AbsoluteTime _time, int *tzp, struct tm *tm, char **tzn)

* Copy no more than MAXTZLEN bytes of timezone to tzn, in case it

* contains an error message, which doesn't fit in the buffer

- StrNCpy(*tzn, tm->tm_zone, MAXTZLEN + 1);

+ strlcpy(*tzn, tm->tm_zone, MAXTZLEN + 1);

if (strlen(tm->tm_zone) > MAXTZLEN)

tm->tm_isdst = -1;

}

@@ -1033,7 +1033,7 @@ abstime2tm(AbsoluteTime _time, int *tzp, struct tm *tm, char **tzn)

* Copy no more than MAXTZLEN bytes of timezone to tzn, in case it

* contains an error message, which doesn't fit in the buffer

- StrNCpy(*tzn, TZNAME_GLOBAL[tm->tm_isdst], MAXTZLEN + 1);

+ strlcpy(*tzn, TZNAME_GLOBAL[tm->tm_isdst], MAXTZLEN + 1);

if (strlen(TZNAME_GLOBAL[tm->tm_isdst]) > MAXTZLEN)

tm->tm_isdst = -1;

}

diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c

index 956a599fcbbce45c2648283917bbcf8bdfe8f05e..46b9e78fe59d4c582e6ea95aa3d7c5fadd430996 100644 (file)

--- a/src/interfaces/ecpg/test/pg_regress_ecpg.c

+++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c

@@ -63,7 +63,7 @@ ecpg_filter(const char *sourcefile, const char *outfile)

if (plen > 1)

{

n = (char *) malloc(plen);

- StrNCpy(n, p + 1, plen);

+ strlcpy(n, p + 1, plen);

replace_string(linebuf, n, "");

}

diff --git a/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c b/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c

index 563ff144cc1093a9a644cd0821e39c35d337e7b7..6b0a3db104c2a21a11decd043700e29bb9750049 100644 (file)

--- a/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c

+++ b/src/test/modules/ssl_passphrase_callback/ssl_passphrase_func.c

@@ -74,7 +74,7 @@ rot13_passphrase(char *buf, int size, int rwflag, void *userdata)

{

Assert(ssl_passphrase != NULL);

- StrNCpy(buf, ssl_passphrase, size);

+ strlcpy(buf, ssl_passphrase, size);

for (char *p = buf; *p; p++)

{

char c = *p;

This is the main PostgreSQL git repository.

RSS Atom