public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amul Sul <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Cleanup: Replace sscanf with strtol/strtoul in snapmgr
Date: Mon, 20 Apr 2026 10:36:21 +0530
Message-ID: <CAAJ_b97mG=6ybUYE8nQyDfKkhaxTCD8rE8s6L8efNmqHaUYNWQ@mail.gmail.com> (raw)

Hi,

The attached patch replaces sscanf with strtol and strtoul in the
ImportSnapshot helpers (parseIntFromText, parseXidFromText, and
parseVxidFromText) to improve reliability and efficiency. By utilizing
the end pointer, we can locate the next line without re-scanning the
entire string.

Additionally, this change aligns the snapshot code with the rest of
the Postgres backend, which already favors these functions for safer
parsing.

--
Regards,
Amul Sul
EDB: http://www.enterprisedb.com


Attachments:

  [application/x-patch] 0001-snapmgr-replace-sscanf-with-strtol-strtoul-in-snapsh.patch (4.1K, 2-0001-snapmgr-replace-sscanf-with-strtol-strtoul-in-snapsh.patch)
  download | inline diff:
From 9869b9f19775de7d13f1bada5db5d8f14bf20859 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Mon, 20 Apr 2026 10:21:28 +0530
Subject: [PATCH] snapmgr: replace sscanf with strtol/strtoul in snapshot parse
 helpers

parseIntFromText, parseXidFromText, and parseVxidFromText used sscanf
to convert digit strings to integers.  Replace with strtol/strtoul,
which provide explicit overflow detection and advance the end-pointer
directly, avoiding a redundant strchr scan over the already-parsed
digits.
---
 src/backend/utils/time/snapmgr.c | 45 +++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 10fe18df2e7..20b8421b881 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1308,24 +1308,27 @@ parseIntFromText(const char *prefix, char **s, const char *filename)
 {
 	char	   *ptr = *s;
 	int			prefixlen = strlen(prefix);
-	int			val;
+	long		val;
+	char	   *endptr;
 
 	if (strncmp(ptr, prefix, prefixlen) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
 	ptr += prefixlen;
-	if (sscanf(ptr, "%d", &val) != 1)
+	errno = 0;
+	val = strtol(ptr, &endptr, 10);
+	if (endptr == ptr || errno != 0 || val < INT_MIN || val > INT_MAX)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
-	ptr = strchr(ptr, '\n');
+	ptr = strchr(endptr, '\n');
 	if (!ptr)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
 	*s = ptr + 1;
-	return val;
+	return (int) val;
 }
 
 static TransactionId
@@ -1333,24 +1336,27 @@ parseXidFromText(const char *prefix, char **s, const char *filename)
 {
 	char	   *ptr = *s;
 	int			prefixlen = strlen(prefix);
-	TransactionId val;
+	unsigned long val;
+	char	   *endptr;
 
 	if (strncmp(ptr, prefix, prefixlen) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
 	ptr += prefixlen;
-	if (sscanf(ptr, "%u", &val) != 1)
+	errno = 0;
+	val = strtoul(ptr, &endptr, 10);
+	if (endptr == ptr || errno != 0 || val > UINT_MAX)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
-	ptr = strchr(ptr, '\n');
+	ptr = strchr(endptr, '\n');
 	if (!ptr)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
 	*s = ptr + 1;
-	return val;
+	return (TransactionId) val;
 }
 
 static void
@@ -1359,17 +1365,36 @@ parseVxidFromText(const char *prefix, char **s, const char *filename,
 {
 	char	   *ptr = *s;
 	int			prefixlen = strlen(prefix);
+	long		lval;
+	unsigned long ulval;
+	char	   *endptr;
 
 	if (strncmp(ptr, prefix, prefixlen) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
 	ptr += prefixlen;
-	if (sscanf(ptr, "%d/%u", &vxid->procNumber, &vxid->localTransactionId) != 2)
+
+	/* Parse procNumber (the signed integer before '/') */
+	errno = 0;
+	lval = strtol(ptr, &endptr, 10);
+	if (endptr == ptr || errno != 0 || lval < INT_MIN || lval > INT_MAX ||
+		*endptr != '/')
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
 				 errmsg("invalid snapshot data in file \"%s\"", filename)));
-	ptr = strchr(ptr, '\n');
+	vxid->procNumber = (ProcNumber) lval;
+	ptr = endptr + 1;			/* skip the '/' separator */
+
+	/* Parse localTransactionId (the unsigned integer after '/') */
+	errno = 0;
+	ulval = strtoul(ptr, &endptr, 10);
+	if (endptr == ptr || errno != 0 || ulval > UINT_MAX)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+				 errmsg("invalid snapshot data in file \"%s\"", filename)));
+	vxid->localTransactionId = (LocalTransactionId) ulval;
+	ptr = strchr(endptr, '\n');
 	if (!ptr)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
-- 
2.47.1



view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Cleanup: Replace sscanf with strtol/strtoul in snapmgr
  In-Reply-To: <CAAJ_b97mG=6ybUYE8nQyDfKkhaxTCD8rE8s6L8efNmqHaUYNWQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox