On 2016-10-03 14:55:24 -0700, Andres Freund wrote:
> Hi,
>
> A colleage of me just wrote innocent looking code like
> char *shardRelationName = pnstrdup(relationName, NAMEDATALEN);
> which is at the moment wrong if relationName isn't preallocated to
> NAMEDATALEN size.
>
> /*
> * pnstrdup
> * Like pstrdup(), but append null byte to a
> * not-necessarily-null-terminated input string.
> */
> char *
> pnstrdup(const char *in, Size len)
> {
> char *out = palloc(len + 1);
>
> memcpy(out, in, len);
> out[len] = '\0';
> return out;
> }
>
> isn't that a somewhat weird behaviour / implementation? Not really like
> strndup(), which one might believe to be analoguous...
I've since hit this bug again. To fix it, you'd need strnlen. The lack
of which I'd also independently hit twice. So here's a patch adding
pg_strnlen and using that to fix pnstrdup.
Greetings,
Andres Freund
>From 89ac4ce2cdad83806f83c0bc5ddac0e9ab1e038c Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 21 Sep 2017 11:43:26 -0700
Subject: [PATCH 1/2] Add pg_strnlen() a portable implementation of strlen.
As the OS version is likely going to be more optimized, fall back to
it if available, as detected by configure.
---
configure | 2 +-
configure.in | 2 +-
src/common/string.c | 20 ++++++++++++++++++++
src/include/common/string.h | 15 +++++++++++++++
src/include/pg_config.h.in | 3 +++
src/include/pg_config.h.win32 | 3 +++
src/port/snprintf.c | 12 ++----------
7 files changed, 45 insertions(+), 12 deletions(-)
diff --git a/configure b/configure
index 5fa7a61025..6584e47293 100755
--- a/configure
+++ b/configure
@@ -8777,7 +8777,7 @@ fi
-for ac_func in strerror_r getpwuid_r gethostbyname_r
+for ac_func in strerror_r getpwuid_r gethostbyname_r strnlen
do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
diff --git a/configure.in b/configure.in
index bebbd11af9..15d4c85162 100644
--- a/configure.in
+++ b/configure.in
@@ -961,7 +961,7 @@ LIBS="$LIBS $PTHREAD_LIBS"
AC_CHECK_HEADER(pthread.h, [], [AC_MSG_ERROR([
pthread.h not found; use --disable-thread-safety to disable thread safety])])
-AC_CHECK_FUNCS([strerror_r getpwuid_r gethostbyname_r])
+AC_CHECK_FUNCS([strerror_r getpwuid_r gethostbyname_r strnlen])
# Do test here with the proper thread flags
PGAC_FUNC_STRERROR_R_INT
diff --git a/src/common/string.c b/src/common/string.c
index 159d9ea7b6..901821f3d8 100644
--- a/src/common/string.c
+++ b/src/common/string.c
@@ -41,3 +41,23 @@ pg_str_endswith(const char *str, const char *end)
str += slen - elen;
return strcmp(str, end) == 0;
}
+
+
+/*
+ * Portable version of posix' strnlen.
+ *
+ * Returns the number of characters before a null-byte in the string pointed
+ * to by str, unless there's no null-byte before maxlen. In the latter case
+ * maxlen is returned.
+ */
+#ifndef HAVE_STRNLEN
+size_t
+pg_strnlen(const char *str, size_t maxlen)
+{
+ const char *p = str;
+
+ while (maxlen-- > 0 && *p)
+ p++;
+ return p - str;
+}
+#endif
diff --git a/src/include/common/string.h b/src/include/common/string.h
index 5f3ea71d61..3d46b80918 100644
--- a/src/include/common/string.h
+++ b/src/include/common/string.h
@@ -12,4 +12,19 @@
extern bool pg_str_endswith(const char *str, const char *end);
+/*
+ * Portable version of posix' strnlen.
+ *
+ * Returns the number of characters before a null-byte in the string pointed
+ * to by str, unless there's no null-byte before maxlen. In the latter case
+ * maxlen is returned.
+ *
+ * Use the system strnlen if provided, it's likely to be faster.
+ */
+#ifdef HAVE_STRNLEN
+#define pg_strnlen(str, maxlen) strnlen(str, maxlen)
+#else
+extern size_t pg_strnlen(const char *str, size_t maxlen);
+#endif
+
#endif /* COMMON_STRING_H */
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index b7ae9a0702..257262908c 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -496,6 +496,9 @@
/* Define to 1 if you have the `strlcpy' function. */
#undef HAVE_STRLCPY
+/* Define to 1 if you have the `strnlen' function. */
+#undef HAVE_STRNLEN
+
/* Define to use have a strong random number source */
#undef HAVE_STRONG_RANDOM
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index e6b3c5d551..08ae2f9a86 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -345,6 +345,9 @@
/* Define to 1 if you have the <string.h> header file. */
#define HAVE_STRING_H 1
+/* Define to 1 if you have the `strnlen' function. */
+#define HAVE_STRNLEN
+
/* Define to use have a strong random number source */
#define HAVE_STRONG_RANDOM 1
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 231e5d6bdb..531d2c5ee3 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -43,6 +43,8 @@
#endif
#include <sys/param.h>
+#include "common/string.h"
+
#ifndef NL_ARGMAX
#define NL_ARGMAX 16
#endif
@@ -790,16 +792,6 @@ bad_format:
target->failed = true;
}
-static size_t
-pg_strnlen(const char *str, size_t maxlen)
-{
- const char *p = str;
-
- while (maxlen-- > 0 && *p)
- p++;
- return p - str;
-}
-
static void
fmtstr(char *value, int leftjust, int minlen, int maxwidth,
int pointflag, PrintfTarget *target)
--
2.14.1.536.g6867272d5b.dirty
>From d3c677e78e98577be101003722fcbcbdf772f3c4 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 21 Sep 2017 11:46:05 -0700
Subject: [PATCH 2/2] Fix pnstrdup() to not memcpy() the maximum allowed
length.
The previous behaviour was dangerous if the length passed wasn't the
size of the underlying buffer, but the maximum size of the underlying
buffer.
Author: Andres Freund
Discussion: https://postgr.es/m/20161003215524.mwz5p45pcverr...@alap3.anarazel.de
---
src/backend/utils/mmgr/mcxt.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index cd696f16bc..64e0408d5a 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -21,6 +21,7 @@
#include "postgres.h"
+#include "common/string.h"
#include "miscadmin.h"
#include "utils/memdebug.h"
#include "utils/memutils.h"
@@ -1086,10 +1087,14 @@ pstrdup(const char *in)
char *
pnstrdup(const char *in, Size len)
{
- char *out = palloc(len + 1);
+ char *out;
+ len = pg_strnlen(in, len);
+
+ out = palloc(len + 1);
memcpy(out, in, len);
out[len] = '\0';
+
return out;
}
--
2.14.1.536.g6867272d5b.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
