[17+] check after second call to pg_strnxfrm is too strict, relax it

Started by Jeff Davisover 1 year ago3 messages
#1Jeff Davis
pgsql@j-davis.com
1 attachment(s)

I noticed this comment in selfuncs.c regarding strxfrm():

/*
* Some systems (e.g., glibc) can return a smaller value from the
* second call than the first; thus the Assert must be <= not ==.
*/

Some callers of pg_strnxfrm() are not allowing for that possibility.

Patch attached, which should be backported to 17.

Regards,
Jeff Davis

Attachments:

0001-Relax-check-for-return-value-from-second-call-of-pg_.patchtext/x-patch; charset=UTF-8; name=0001-Relax-check-for-return-value-from-second-call-of-pg_.patchDownload
From 9a9680455cab0cf747ef65cf11cf2cb06d5f500e Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Tue, 30 Jul 2024 12:16:45 -0700
Subject: [PATCH] Relax check for return value from second call of
 pg_strnxfrm().

strxfrm() is not guaranteed to return the exact number of bytes needed
to store the result; it may return a higher value.
---
 src/backend/access/hash/hashfunc.c |  4 ++--
 src/backend/utils/adt/pg_locale.c  | 12 ++++++------
 src/backend/utils/adt/varchar.c    |  4 ++--
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/backend/access/hash/hashfunc.c b/src/backend/access/hash/hashfunc.c
index ce8ee0ea2ef..9d9a16467b4 100644
--- a/src/backend/access/hash/hashfunc.c
+++ b/src/backend/access/hash/hashfunc.c
@@ -298,7 +298,7 @@ hashtext(PG_FUNCTION_ARGS)
 		buf = palloc(bsize + 1);
 
 		rsize = pg_strnxfrm(buf, bsize + 1, keydata, keylen, mylocale);
-		if (rsize != bsize)
+		if (rsize > bsize)
 			elog(ERROR, "pg_strnxfrm() returned unexpected result");
 
 		/*
@@ -352,7 +352,7 @@ hashtextextended(PG_FUNCTION_ARGS)
 		buf = palloc(bsize + 1);
 
 		rsize = pg_strnxfrm(buf, bsize + 1, keydata, keylen, mylocale);
-		if (rsize != bsize)
+		if (rsize > bsize)
 			elog(ERROR, "pg_strnxfrm() returned unexpected result");
 
 		/*
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 627ab89d7cc..41330077cfa 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2353,9 +2353,9 @@ pg_strxfrm_enabled(pg_locale_t locale)
  * The provided 'src' must be nul-terminated. If 'destsize' is zero, 'dest'
  * may be NULL.
  *
- * Returns the number of bytes needed to store the transformed string,
- * excluding the terminating nul byte. If the value returned is 'destsize' or
- * greater, the resulting contents of 'dest' are undefined.
+ * Returns the number of bytes (or more) needed to store the transformed
+ * string, excluding the terminating nul byte. If the value returned is
+ * 'destsize' or greater, the resulting contents of 'dest' are undefined.
  */
 size_t
 pg_strxfrm(char *dest, const char *src, size_t destsize, pg_locale_t locale)
@@ -2385,9 +2385,9 @@ pg_strxfrm(char *dest, const char *src, size_t destsize, pg_locale_t locale)
  * 'src' does not need to be nul-terminated. If 'destsize' is zero, 'dest' may
  * be NULL.
  *
- * Returns the number of bytes needed to store the transformed string,
- * excluding the terminating nul byte. If the value returned is 'destsize' or
- * greater, the resulting contents of 'dest' are undefined.
+ * Returns the number of bytes (or more) needed to store the transformed
+ * string, excluding the terminating nul byte. If the value returned is
+ * 'destsize' or greater, the resulting contents of 'dest' are undefined.
  *
  * This function may need to nul-terminate the argument for libc functions;
  * so if the caller already has a nul-terminated string, it should call
diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 02dfe219f54..b5cc5c4792e 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -1028,7 +1028,7 @@ hashbpchar(PG_FUNCTION_ARGS)
 		buf = palloc(bsize + 1);
 
 		rsize = pg_strnxfrm(buf, bsize + 1, keydata, keylen, mylocale);
-		if (rsize != bsize)
+		if (rsize > bsize)
 			elog(ERROR, "pg_strnxfrm() returned unexpected result");
 
 		/*
@@ -1084,7 +1084,7 @@ hashbpcharextended(PG_FUNCTION_ARGS)
 		buf = palloc(bsize + 1);
 
 		rsize = pg_strnxfrm(buf, bsize + 1, keydata, keylen, mylocale);
-		if (rsize != bsize)
+		if (rsize > bsize)
 			elog(ERROR, "pg_strnxfrm() returned unexpected result");
 
 		/*
-- 
2.34.1

#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Jeff Davis (#1)
Re: [17+] check after second call to pg_strnxfrm is too strict, relax it

On 30/07/2024 22:31, Jeff Davis wrote:

I noticed this comment in selfuncs.c regarding strxfrm():

/*
* Some systems (e.g., glibc) can return a smaller value from the
* second call than the first; thus the Assert must be <= not ==.
*/

Some callers of pg_strnxfrm() are not allowing for that possibility.

Patch attached, which should be backported to 17.

+1. A comment in the pg_strnxfrm() function itself would be good too.

--
Heikki Linnakangas
Neon (https://neon.tech)

#3Jeff Davis
pgsql@j-davis.com
In reply to: Heikki Linnakangas (#2)
Re: [17+] check after second call to pg_strnxfrm is too strict, relax it

On Tue, 2024-07-30 at 23:01 +0300, Heikki Linnakangas wrote:

+1. A comment in the pg_strnxfrm() function itself would be good too.

Committed, thank you. Backported to 16 (original email said 17, but
that was a mistake).

Regards,
Jeff Davis