v2-0001-Replace-remaining-StrNCpy-by-strlcpy.patch

From 740866001613e9d6aa29f759085ea284c0d41c94 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 4 Aug 2020 15:16:37 +0200
Subject: [PATCH v2] Replace remaining StrNCpy() by strlcpy()

They are equivalent, and strlcpy() has become the preferred spelling.
Convert over the remaining cases.  Remove StrNCpy() as it is now
unused.
---
 contrib/pgcrypto/crypt-des.c                  |  2 +-
 src/backend/access/transam/slru.c             |  2 +-
 src/backend/access/transam/xlogarchive.c      |  2 +-
 src/backend/catalog/pg_constraint.c           |  2 +-
 src/backend/commands/indexcmds.c              |  2 +-
 src/backend/commands/statscmds.c              |  2 +-
 src/backend/commands/tablecmds.c              |  2 +-
 src/backend/postmaster/pgstat.c               |  2 +-
 src/backend/replication/logical/logical.c     |  3 +-
 src/backend/replication/slot.c                |  2 +-
 src/backend/utils/adt/formatting.c            |  8 ++---
 src/backend/utils/adt/name.c                  |  6 ++--
 src/backend/utils/adt/pg_locale.c             |  9 ------
 src/backend/utils/adt/ruleutils.c             |  2 +-
 src/common/exec.c                             |  4 +--
 src/include/c.h                               | 29 -------------------
 src/interfaces/ecpg/pgtypeslib/dt_common.c    |  4 +--
 src/interfaces/ecpg/test/pg_regress_ecpg.c    |  2 +-
 .../ssl_passphrase_func.c                     |  2 +-
 19 files changed, 26 insertions(+), 61 deletions(-)

diff --git a/contrib/pgcrypto/crypt-des.c b/contrib/pgcrypto/crypt-des.c
index 6efaa609c9..98c30ea122 100644
--- 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 9e145f1c36..d1dbb43e09 100644
--- 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 cdd586fcfb..8f8734dc1d 100644
--- 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 fdc63e7dea..6a6b2cb8c0 100644
--- 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 2baca12c5f..7819266a63 100644
--- 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 974828545c..3057d89d50 100644
--- 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 ac53f79ada..cd989c95e5 100644
--- 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 15f92b66c6..73ce944fb1 100644
--- 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 05d24b93da..ebc0b7080e 100644
--- 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 */
@@ -321,7 +322,7 @@ CreateInitDecodingContext(char *plugin,
 
 	/* register output plugin name with slot */
 	SpinLockAcquire(&slot->mutex);
-	StrNCpy(NameStr(slot->data.plugin), plugin, NAMEDATALEN);
+	namestrcpy(&slot->data.plugin, plugin);
 	SpinLockRelease(&slot->mutex);
 
 	if (XLogRecPtrIsInvalid(restart_lsn))
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 57bbb6288c..3dc01b6df2 100644
--- 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 6626438136..9de63686ec 100644
--- 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 64877f67e0..895def56c2 100644
--- a/src/backend/utils/adt/name.c
+++ b/src/backend/utils/adt/name.c
@@ -234,7 +234,7 @@ namecpy(Name n1, const NameData *n2)
 {
 	if (!n1 || !n2)
 		return -1;
-	StrNCpy(NameStr(*n1), NameStr(*n2), NAMEDATALEN);
+	memcpy(n1, n2, NAMEDATALEN);
 	return 0;
 }
 
@@ -251,7 +251,9 @@ namestrcpy(Name name, const char *str)
 {
 	if (!name || !str)
 		return -1;
-	StrNCpy(NameStr(*name), str, NAMEDATALEN);
+	/* NB: We need to zero-pad the destination. */
+	strncpy(NameStr(*name), str, NAMEDATALEN);
+	NameStr(*name)[NAMEDATALEN-1] = '\0';
 	return 0;
 }
 
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 11d05c73ac..07299dbc09 100644
--- 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 2cbcb4b85e..f507e5cd76 100644
--- 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);
+		namecpy(result, &role_rec->rolname);
 		ReleaseSysCache(roletup);
 	}
 	else
diff --git a/src/common/exec.c b/src/common/exec.c
index f39b0a294b..78bb486f99 100644
--- 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 f242e32edb..2c61ca8aa8 100644
--- 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/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c
index 14cdf2d428..e8a8a0f0ed 100644
--- 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 956a599fcb..46b9e78fe5 100644
--- 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 563ff144cc..6b0a3db104 100644
--- 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;
-- 
2.28.0