On 2019-Dec-04, Alvaro Herrera wrote:
>
> > (Maybe do strnlen(maxlen), then count strnlen(1) starting at that point
> > -- so if that returns >=1, print the "..."?)
>
> So I found that I can make the code more reasonable with this simple
> coding,
With strndup.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 9a92076e53a6664ec61c5f475310c5d6edb91d90 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Wed, 4 Dec 2019 11:02:34 -0300
Subject: [PATCH v3 1/3] Add strndup / pg_strndup
The former is now a POSIX function. We already had pnstrdup, but it was
not offered to frontend code.
---
configure | 23 +++++++++++++++++++++++
configure.in | 3 ++-
src/common/fe_memutils.c | 26 ++++++++++++++++++++++++++
src/include/common/fe_memutils.h | 2 ++
src/include/pg_config.h.in | 7 +++++++
src/include/pg_config.h.win32 | 7 +++++++
src/include/port.h | 4 ++++
src/port/strndup.c | 32 ++++++++++++++++++++++++++++++++
8 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 src/port/strndup.c
diff --git a/configure b/configure
index 1d88983b34..fd8c759473 100755
--- a/configure
+++ b/configure
@@ -15482,6 +15482,16 @@ fi
cat >>confdefs.h <<_ACEOF
#define HAVE_DECL_STRLCPY $ac_have_decl
_ACEOF
+ac_fn_c_check_decl "$LINENO" "strndup" "ac_cv_have_decl_strndup" "$ac_includes_default"
+if test "x$ac_cv_have_decl_strndup" = xyes; then :
+ ac_have_decl=1
+else
+ ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_STRNDUP $ac_have_decl
+_ACEOF
ac_fn_c_check_decl "$LINENO" "strnlen" "ac_cv_have_decl_strnlen" "$ac_includes_default"
if test "x$ac_cv_have_decl_strnlen" = xyes; then :
ac_have_decl=1
@@ -15815,6 +15825,19 @@ esac
fi
+ac_fn_c_check_func "$LINENO" "strndup" "ac_cv_func_strndup"
+if test "x$ac_cv_func_strndup" = xyes; then :
+ $as_echo "#define HAVE_STRNDUP 1" >>confdefs.h
+
+else
+ case " $LIBOBJS " in
+ *" strndup.$ac_objext "* ) ;;
+ *) LIBOBJS="$LIBOBJS strndup.$ac_objext"
+ ;;
+esac
+
+fi
+
ac_fn_c_check_func "$LINENO" "strnlen" "ac_cv_func_strnlen"
if test "x$ac_cv_func_strnlen" = xyes; then :
$as_echo "#define HAVE_STRNLEN 1" >>confdefs.h
diff --git a/configure.in b/configure.in
index a2cb20b5e3..11f66d19c3 100644
--- a/configure.in
+++ b/configure.in
@@ -1679,7 +1679,7 @@ AC_CHECK_DECLS(posix_fadvise, [], [], [#include <fcntl.h>])
]) # fi
AC_CHECK_DECLS(fdatasync, [], [], [#include <unistd.h>])
-AC_CHECK_DECLS([strlcat, strlcpy, strnlen])
+AC_CHECK_DECLS([strlcat, strlcpy, strndup, strnlen])
# This is probably only present on macOS, but may as well check always
AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include <fcntl.h>])
@@ -1738,6 +1738,7 @@ AC_REPLACE_FUNCS(m4_normalize([
srandom
strlcat
strlcpy
+ strndup
strnlen
strtof
]))
diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index ce99b4f4da..896e5d46ca 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -101,6 +101,26 @@ pg_strdup(const char *in)
return tmp;
}
+char *
+pg_strndup(const char *in, size_t size)
+{
+ char *tmp;
+
+ if (!in)
+ {
+ fprintf(stderr,
+ _("cannot duplicate null pointer (internal error)\n"));
+ exit(EXIT_FAILURE);
+ }
+ tmp = strndup(in, size);
+ if (!tmp)
+ {
+ fprintf(stderr, _("out of memory\n"));
+ exit(EXIT_FAILURE);
+ }
+ return tmp;
+}
+
void
pg_free(void *ptr)
{
@@ -142,6 +162,12 @@ pstrdup(const char *in)
return pg_strdup(in);
}
+char *
+pnstrdup(const char *in, Size size)
+{
+ return pg_strndup(in, size);
+}
+
void *
repalloc(void *pointer, Size size)
{
diff --git a/src/include/common/fe_memutils.h b/src/include/common/fe_memutils.h
index a1e5940d31..c8797ffe38 100644
--- a/src/include/common/fe_memutils.h
+++ b/src/include/common/fe_memutils.h
@@ -23,6 +23,7 @@
* (except pg_malloc_extended with MCXT_ALLOC_NO_OOM)
*/
extern char *pg_strdup(const char *in);
+extern char *pg_strndup(const char *in, size_t size);
extern void *pg_malloc(size_t size);
extern void *pg_malloc0(size_t size);
extern void *pg_malloc_extended(size_t size, int flags);
@@ -31,6 +32,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/include/pg_config.h.in b/src/include/pg_config.h.in
index c208dcdfc7..1db3f1b1e8 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -170,6 +170,10 @@
don't. */
#undef HAVE_DECL_STRLCPY
+/* Define to 1 if you have the declaration of `strndup', and to 0 if you
+ don't. */
+#undef HAVE_DECL_STRNDUP
+
/* Define to 1 if you have the declaration of `strnlen', and to 0 if you
don't. */
#undef HAVE_DECL_STRNLEN
@@ -545,6 +549,9 @@
/* Define to 1 if you have the `strlcpy' function. */
#undef HAVE_STRLCPY
+/* Define to 1 if you have the `strndup' function. */
+#undef HAVE_STRNDUP
+
/* Define to 1 if you have the `strnlen' function. */
#undef HAVE_STRNLEN
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 6c98ef4916..2ff2c3d4a8 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -129,6 +129,10 @@
don't. */
#define HAVE_DECL_RTLD_NOW 0
+/* Define to 1 if you have the declaration of `strndup', and to 0 if you
+ don't. */
+#undef HAVE_DECL_STRNDUP
+
/* Define to 1 if you have the declaration of `strnlen', and to 0 if you
don't. */
#define HAVE_DECL_STRNLEN 1
@@ -297,6 +301,9 @@
/* Define to 1 if you have the <pam/pam_appl.h> header file. */
/* #undef HAVE_PAM_PAM_APPL_H */
+/* Define to 1 if you have the `strndup' function. */
+#undef HAVE_STRNDUP
+
/* Define to 1 if you have the `strnlen' function. */
#define HAVE_STRNLEN 1
diff --git a/src/include/port.h b/src/include/port.h
index 10dcb5f0a6..e5d4486eb2 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -430,6 +430,10 @@ extern size_t strlcat(char *dst, const char *src, size_t siz);
extern size_t strlcpy(char *dst, const char *src, size_t siz);
#endif
+#if !HAVE_DECL_STRNDUP
+extern char *strndup(char *src, size_t siz);
+#endif
+
#if !HAVE_DECL_STRNLEN
extern size_t strnlen(const char *str, size_t maxlen);
#endif
diff --git a/src/port/strndup.c b/src/port/strndup.c
new file mode 100644
index 0000000000..eba41a0498
--- /dev/null
+++ b/src/port/strndup.c
@@ -0,0 +1,32 @@
+/*
+ * strndup.c
+ * Fallback implementation of strndup().
+ *
+ * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * IDENTIFICATION
+ * src/port/strndup.c
+ */
+
+#include "c.h"
+
+/*
+ * Implementation of posix' strndup for systems where it's not available.
+ */
+char *
+strndup(const char *str, size_t siz)
+{
+ char *dst;
+ int len;
+
+ len = strnlen(str, siz);
+ dst = malloc(len + 1);
+ if (dst == NULL)
+ return NULL;
+
+ memcpy(dst, str, len);
+ dst[len + 1] = '\0';
+
+ return dst;
+}
--
2.20.1
>From c8eea42dcd441fdf0c7ded15538bd113dfbf0ff7 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Tue, 3 Dec 2019 10:08:35 -0300
Subject: [PATCH v3 2/3] Add appendStringInfoStringQuoted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Simplifies some coding that prints parameters, as well as optimize to do
it per non-quote chunks instead of per byte.
Author: Alexey Bashtanov and Álvaro Herrera, after a suggestion from Andres Freund
Discussion: https://postgr.es/m/20190920203905.xkv5udsd5dxfs...@alap3.anarazel.de
---
src/backend/tcop/postgres.c | 10 +--------
src/common/stringinfo.c | 40 ++++++++++++++++++++++++++++++++++++
src/include/lib/stringinfo.h | 7 +++++++
src/pl/plpgsql/src/pl_exec.c | 34 ++++++++----------------------
4 files changed, 56 insertions(+), 35 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 3b85e48333..0bff20ad67 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2348,7 +2348,6 @@ errdetail_params(ParamListInfo params)
Oid typoutput;
bool typisvarlena;
char *pstring;
- char *p;
appendStringInfo(¶m_str, "%s$%d = ",
paramno > 0 ? ", " : "",
@@ -2364,14 +2363,7 @@ errdetail_params(ParamListInfo params)
pstring = OidOutputFunctionCall(typoutput, prm->value);
- appendStringInfoCharMacro(¶m_str, '\'');
- for (p = pstring; *p; p++)
- {
- if (*p == '\'') /* double single quotes */
- appendStringInfoCharMacro(¶m_str, *p);
- appendStringInfoCharMacro(¶m_str, *p);
- }
- appendStringInfoCharMacro(¶m_str, '\'');
+ appendStringInfoStringQuoted(¶m_str, pstring);
pfree(pstring);
}
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index a50e587da9..f4dddf8223 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -178,6 +178,46 @@ appendStringInfoString(StringInfo str, const char *s)
appendBinaryStringInfo(str, s, strlen(s));
}
+/*
+ * appendStringInfoStringQuoted
+ *
+ * Append a null-terminated string to str, adding single quotes around it
+ * and doubling all single quotes.
+ */
+void
+appendStringInfoStringQuoted(StringInfo str, const char *s)
+{
+ const char *chunk_search_start = s,
+ *chunk_copy_start = s,
+ *chunk_end;
+ int len;
+
+ Assert(str != NULL);
+
+ appendStringInfoCharMacro(str, '\'');
+
+ while ((chunk_end = strchr(chunk_search_start, '\'')) != NULL)
+ {
+ /* copy including the found delimiting ' */
+ appendBinaryStringInfoNT(str,
+ chunk_copy_start,
+ chunk_end - chunk_copy_start + 1);
+
+ /* in order to double it, include this ' into the next chunk as well */
+ chunk_copy_start = chunk_end;
+ chunk_search_start = chunk_end + 1;
+ }
+
+ /* copy the last chunk, ensuring sufficient space for final bytes */
+ len = strlen(chunk_copy_start);
+ enlargeStringInfo(str, len + 2);
+ appendBinaryStringInfoNT(str, chunk_copy_start, len);
+
+ /* add terminators */
+ appendStringInfoCharMacro(str, '\'');
+ str->data[str->len] = '\0';
+}
+
/*
* appendStringInfoChar
*
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index e27942728e..7de2773e1f 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -113,6 +113,13 @@ extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_
*/
extern void appendStringInfoString(StringInfo str, const char *s);
+/*------------------------
+ * appendStringInfoStringQuoted
+ * Append a null-terminated string to str, adding single quotes around it
+ * and doubling all single quotes.
+ */
+extern void appendStringInfoStringQuoted(StringInfo str, const char *s);
+
/*------------------------
* appendStringInfoChar
* Append a single byte to str.
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 4f0de7a811..23553a6948 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8611,19 +8611,10 @@ format_expr_params(PLpgSQL_execstate *estate,
if (paramisnull)
appendStringInfoString(¶mstr, "NULL");
else
- {
- char *value = convert_value_to_string(estate, paramdatum, paramtypeid);
- char *p;
-
- appendStringInfoCharMacro(¶mstr, '\'');
- for (p = value; *p; p++)
- {
- if (*p == '\'') /* double single quotes */
- appendStringInfoCharMacro(¶mstr, *p);
- appendStringInfoCharMacro(¶mstr, *p);
- }
- appendStringInfoCharMacro(¶mstr, '\'');
- }
+ appendStringInfoStringQuoted(¶mstr,
+ convert_value_to_string(estate,
+ paramdatum,
+ paramtypeid));
paramno++;
}
@@ -8661,19 +8652,10 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
if (ppd->nulls[paramno] == 'n')
appendStringInfoString(¶mstr, "NULL");
else
- {
- char *value = convert_value_to_string(estate, ppd->values[paramno], ppd->types[paramno]);
- char *p;
-
- appendStringInfoCharMacro(¶mstr, '\'');
- for (p = value; *p; p++)
- {
- if (*p == '\'') /* double single quotes */
- appendStringInfoCharMacro(¶mstr, *p);
- appendStringInfoCharMacro(¶mstr, *p);
- }
- appendStringInfoCharMacro(¶mstr, '\'');
- }
+ appendStringInfoStringQuoted(¶mstr,
+ convert_value_to_string(estate,
+ ppd->values[paramno],
+ ppd->types[paramno]));
}
MemoryContextSwitchTo(oldcontext);
--
2.20.1
>From a34b4728658891cb7d37433877efd0b706331dd3 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@alvh.no-ip.org>
Date: Tue, 3 Dec 2019 18:12:25 -0300
Subject: [PATCH v3 3/3] Restrict length printed by
appendStringInfoStringQuoted
---
src/backend/tcop/postgres.c | 2 +-
src/common/stringinfo.c | 49 +++++++++++++++++++++------
src/include/lib/stringinfo.h | 3 +-
src/pl/plpgsql/src/pl_exec.c | 6 ++--
src/test/regress/expected/plpgsql.out | 14 ++++++++
src/test/regress/sql/plpgsql.sql | 13 +++++++
6 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 0bff20ad67..c38ea2c743 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2363,7 +2363,7 @@ errdetail_params(ParamListInfo params)
pstring = OidOutputFunctionCall(typoutput, prm->value);
- appendStringInfoStringQuoted(¶m_str, pstring);
+ appendStringInfoStringQuoted(¶m_str, pstring, 64);
pfree(pstring);
}
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index f4dddf8223..6f7555ef96 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -185,15 +185,34 @@ appendStringInfoString(StringInfo str, const char *s)
* and doubling all single quotes.
*/
void
-appendStringInfoStringQuoted(StringInfo str, const char *s)
+appendStringInfoStringQuoted(StringInfo str, const char *s, int maxlen)
{
- const char *chunk_search_start = s,
- *chunk_copy_start = s,
+ char *copy = NULL;
+ const char *chunk_search_start,
+ *chunk_copy_start,
*chunk_end;
- int len;
+ bool ellipsis;
Assert(str != NULL);
+ if (maxlen > 0)
+ {
+ copy = pnstrdup(s, maxlen);
+ chunk_search_start = copy;
+ chunk_copy_start = copy;
+
+ ellipsis = strnlen(s, maxlen + 1) > maxlen;
+ /* enlarge while we can do so cheaply */
+ enlargeStringInfo(str, maxlen);
+ }
+ else
+ {
+ chunk_search_start = s;
+ chunk_copy_start = s;
+
+ ellipsis = false;
+ }
+
appendStringInfoCharMacro(str, '\'');
while ((chunk_end = strchr(chunk_search_start, '\'')) != NULL)
@@ -208,14 +227,22 @@ appendStringInfoStringQuoted(StringInfo str, const char *s)
chunk_search_start = chunk_end + 1;
}
- /* copy the last chunk, ensuring sufficient space for final bytes */
- len = strlen(chunk_copy_start);
- enlargeStringInfo(str, len + 2);
- appendBinaryStringInfoNT(str, chunk_copy_start, len);
+ /* copy the last chunk and terminate */
+ if (ellipsis)
+ appendStringInfo(str, "%s...'", chunk_copy_start);
+ else
+ {
+ int len = strlen(chunk_copy_start);
- /* add terminators */
- appendStringInfoCharMacro(str, '\'');
- str->data[str->len] = '\0';
+ /* ensure sufficient space for terminators */
+ enlargeStringInfo(str, len + 2);
+ appendBinaryStringInfoNT(str, chunk_copy_start, len);
+ appendStringInfoCharMacro(str, '\'');
+ str->data[str->len] = '\0';
+ }
+
+ if (copy)
+ pfree(copy);
}
/*
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 7de2773e1f..956ccbba45 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -118,7 +118,8 @@ extern void appendStringInfoString(StringInfo str, const char *s);
* Append a null-terminated string to str, adding single quotes around it
* and doubling all single quotes.
*/
-extern void appendStringInfoStringQuoted(StringInfo str, const char *s);
+extern void appendStringInfoStringQuoted(StringInfo str, const char *s,
+ int maxlen);
/*------------------------
* appendStringInfoChar
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 23553a6948..752a66f379 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -8614,7 +8614,8 @@ format_expr_params(PLpgSQL_execstate *estate,
appendStringInfoStringQuoted(¶mstr,
convert_value_to_string(estate,
paramdatum,
- paramtypeid));
+ paramtypeid),
+ 64);
paramno++;
}
@@ -8655,7 +8656,8 @@ format_preparedparamsdata(PLpgSQL_execstate *estate,
appendStringInfoStringQuoted(¶mstr,
convert_value_to_string(estate,
ppd->values[paramno],
- ppd->types[paramno]));
+ ppd->types[paramno]),
+ 64);
}
MemoryContextSwitchTo(oldcontext);
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index e85b29455e..a53ae28548 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -2656,6 +2656,20 @@ create or replace function stricttest() returns void as $$
declare
x record;
p1 int := 2;
+p3 text := $a$'Valame Dios!' dijo Sancho; 'no le dije yo a vuestra merced que mirase bien lo que hacia?'$a$;
+begin
+ -- no rows
+ select * from foo where f1 = p1 and f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+end$$ language plpgsql;
+select stricttest();
+ERROR: query returned no rows
+DETAIL: parameters: p1 = '2', p3 = '''Valame Dios!'' dijo Sancho; ''no le dije yo a vuestra merced que ...'
+CONTEXT: PL/pgSQL function stricttest() line 8 at SQL statement
+create or replace function stricttest() returns void as $$
+declare
+x record;
+p1 int := 2;
p3 text := 'foo';
begin
-- too many rows
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 70deadfbea..d841d8c0f9 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2280,6 +2280,19 @@ end$$ language plpgsql;
select stricttest();
+create or replace function stricttest() returns void as $$
+declare
+x record;
+p1 int := 2;
+p3 text := $a$'Valame Dios!' dijo Sancho; 'no le dije yo a vuestra merced que mirase bien lo que hacia?'$a$;
+begin
+ -- no rows
+ select * from foo where f1 = p1 and f1::text = p3 into strict x;
+ raise notice 'x.f1 = %, x.f2 = %', x.f1, x.f2;
+end$$ language plpgsql;
+
+select stricttest();
+
create or replace function stricttest() returns void as $$
declare
x record;
--
2.20.1
