Date: Wed, 4 Dec 2019 19:01:46 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: adding strndup
Message-ID: <20191204220146.GA28850@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="vkogqOf2sHV7VnPd"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20191204190501.GA29287@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--vkogqOf2sHV7VnPd
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Dec-04, Alvaro Herrera wrote:

> On 2019-Dec-04, Tom Lane wrote:
> 
> > Andres Freund <andres@anarazel.de> writes:
> > > On 2019-12-04 11:40:21 -0300, Alvaro Herrera wrote:
> > >> I think this should be pretty uncontroversial, but wanted to give a
> > >> heads-up outside that thread.  I attach the patch here for completeness.
> > 
> > > I'd just provide pnstrdup() in the frontend, without adding strndup().
> > 
> > +1 --- seems like a bunch more mechanism than is warranted.  Let's
> > just open-code it in pnstrdup.  We can rely on strnlen, since that's
> > already supported, and there's not much more there beyond that.
> 
> I can get behind that ... it makes the patch a lot smaller.

Here it is.

I noticed that ECPG's copy was setting errno.  I had forgot to do that
in my previous patch, but on second look, malloc failure already sets
it, so doing it again is pointless.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--vkogqOf2sHV7VnPd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v4-0001-Offer-pnstrdup-to-frontend-code.patch"

From c4dc0fd76e86bf97cb612bafe30847c47b77813e Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 4 Dec 2019 11:02:34 -0300
Subject: [PATCH v4] Offer pnstrdup to frontend code

We already had it on the backend.  Frontend can also use it now.

Discussion: https://postgr.es/m/20191204144021.GA17976@alvherre.pgsql
---
 src/bin/pg_waldump/pg_waldump.c          |  3 +--
 src/bin/psql/prompt.c                    | 16 +++++---------
 src/bin/scripts/common.c                 |  3 +--
 src/common/fe_memutils.c                 | 27 ++++++++++++++++++++++++
 src/include/common/fe_memutils.h         |  1 +
 src/interfaces/ecpg/compatlib/informix.c | 21 +-----------------
 6 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 30a5851d87..a05fbe6938 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -114,8 +114,7 @@ split_path(const char *path, char **dir, char **fname)
 	/* directory path */
 	if (sep != NULL)
 	{
-		*dir = pg_strdup(path);
-		(*dir)[(sep - path) + 1] = '\0';	/* no strndup */
+		*dir = pnstrdup(path, sep - path);
 		*fname = pg_strdup(sep + 1);
 	}
 	/* local directory */
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index 41c6f21ecf..1199181521 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -270,13 +270,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 					/* execute command */
 				case '`':
 					{
-						FILE	   *fd;
-						char	   *file = pg_strdup(p + 1);
-						int			cmdend;
+						int			cmdend = strcspn(p + 1, "`");
+						char	   *file = pnstrdup(p + 1, cmdend);
+						FILE	   *fd = popen(file, "r");
 
-						cmdend = strcspn(file, "`");
-						file[cmdend] = '\0';
-						fd = popen(file, "r");
 						if (fd)
 						{
 							if (fgets(buf, sizeof(buf), fd) == NULL)
@@ -295,13 +292,10 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
 					/* interpolate variable */
 				case ':':
 					{
-						char	   *name;
+						int			nameend = strcspn(p + 1, ":");
+						char	   *name = pnstrdup(p + 1, nameend);
 						const char *val;
-						int			nameend;
 
-						name = pg_strdup(p + 1);
-						nameend = strcspn(name, ":");
-						name[nameend] = '\0';
 						val = GetVariable(pset.vars, name);
 						if (val)
 							strlcpy(buf, val, sizeof(buf));
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index d2a7547441..d919c1cc0b 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -352,8 +352,7 @@ splitTableColumnsSpec(const char *spec, int encoding,
 		else
 			cp += PQmblen(cp, encoding);
 	}
-	*table = pg_strdup(spec);
-	(*table)[cp - spec] = '\0'; /* no strndup */
+	*table = pnstrdup(spec, cp - spec);
 	*columns = cp;
 }
 
diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index ce99b4f4da..2bc6606b80 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -142,6 +142,33 @@ pstrdup(const char *in)
 	return pg_strdup(in);
 }
 
+char *
+pnstrdup(const char *in, Size size)
+{
+	char	   *tmp;
+	int			len;
+
+	if (!in)
+	{
+		fprintf(stderr,
+				_("cannot duplicate null pointer (internal error)\n"));
+		exit(EXIT_FAILURE);
+	}
+
+	len = strnlen(in, size);
+	tmp = malloc(len + 1);
+	if (tmp == NULL)
+	{
+		fprintf(stderr, _("out of memory\n"));
+		exit(EXIT_FAILURE);
+	}
+
+	memcpy(tmp, in, len);
+	tmp[len] = '\0';
+
+	return tmp;
+}
+
 void *
 repalloc(void *pointer, Size size)
 {
diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h
index a1e5940d31..3181ee17dd 100644
--- a/src/include/common/fe_memutils.h
+++ b/src/include/common/fe_memutils.h
@@ -31,6 +31,7 @@ extern void pg_free(void *pointer);
 
 /* Equivalent functions, deliberately named the same as backend functions */
 extern char *pstrdup(const char *in);
+extern char *pnstrdup(const char *in, Size size);
 extern void *palloc(Size size);
 extern void *palloc0(Size size);
 extern void *palloc_extended(Size size, int flags);
diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index a7bbeb9223..b2a19a1dd3 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -175,25 +175,6 @@ deccopy(decimal *src, decimal *target)
 	memcpy(target, src, sizeof(decimal));
 }
 
-static char *
-ecpg_strndup(const char *str, size_t len)
-{
-	size_t		real_len = strlen(str);
-	int			use_len = (int) ((real_len > len) ? len : real_len);
-
-	char	   *new = malloc(use_len + 1);
-
-	if (new)
-	{
-		memcpy(new, str, use_len);
-		new[use_len] = '\0';
-	}
-	else
-		errno = ENOMEM;
-
-	return new;
-}
-
 int
 deccvasc(const char *cp, int len, decimal *np)
 {
@@ -205,7 +186,7 @@ deccvasc(const char *cp, int len, decimal *np)
 	if (risnull(CSTRINGTYPE, cp))
 		return 0;
 
-	str = ecpg_strndup(cp, len);	/* decimal_in always converts the complete
+	str = pnstrdup(cp, len);		/* decimal_in always converts the complete
 									 * string */
 	if (!str)
 		ret = ECPG_INFORMIX_NUM_UNDERFLOW;
-- 
2.20.1


--vkogqOf2sHV7VnPd--