public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amul Sul <[email protected]>
To: Tristan Partin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Cleanup: Replace sscanf with strtol/strtoul in snapmgr
Date: Tue, 5 May 2026 12:53:33 +0530
Message-ID: <CAAJ_b974KcAQ=XMBfw-iy2tnbwNk9YnSA5XcQctf7N0GUSDRQA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAAJ_b97mG=6ybUYE8nQyDfKkhaxTCD8rE8s6L8efNmqHaUYNWQ@mail.gmail.com>
<[email protected]>
On Mon, May 4, 2026 at 9:19 PM Tristan Partin <[email protected]> wrote:
>
> On Mon Apr 20, 2026 at 12:07 AM CDT, Amul Sul wrote:
> The patch generally looks good. One comment:
>
> > @@ -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;
>
> Perhaps better variable names would be procNumber and
> localTransactionId.
>
Thanks, Andreas and Tristan, for the review !
I have renamed the variables as suggested but used the shorter forms
procno and xid instead of procNumber and localTransactionId. I also
applied similar changes to parseXidFromText (changing val to xid), but
kept val in parseIntFromText since it seems to be more appropriate for
a generic integer value.
Updated patch attached.
Regards,
Amul
Attachments:
[application/octet-stream] v2-0001-snapmgr-replace-sscanf-with-strtol-strtoul-in-sna.patch (4.1K, 2-v2-0001-snapmgr-replace-sscanf-with-strtol-strtoul-in-sna.patch)
download | inline diff:
From 583dc36426508b73364a681be6ed29e1a4d87808 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Mon, 20 Apr 2026 10:21:28 +0530
Subject: [PATCH v2] 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 | 47 +++++++++++++++++++++++++-------
1 file changed, 37 insertions(+), 10 deletions(-)
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 10fe18df2e7..27a0ec835e8 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1308,24 +1308,28 @@ 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 +1337,27 @@ parseXidFromText(const char *prefix, char **s, const char *filename)
{
char *ptr = *s;
int prefixlen = strlen(prefix);
- TransactionId val;
+ unsigned long xid;
+ 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;
+ xid = strtoul(ptr, &endptr, 10);
+ if (endptr == ptr || errno != 0 || xid > 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) xid;
}
static void
@@ -1359,17 +1366,37 @@ parseVxidFromText(const char *prefix, char **s, const char *filename,
{
char *ptr = *s;
int prefixlen = strlen(prefix);
+ long procno;
+ unsigned long xid;
+ 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;
+ procno = strtol(ptr, &endptr, 10);
+ if (endptr == ptr || errno != 0 ||
+ procno < INT_MIN || procno > 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) procno;
+ ptr = endptr + 1; /* skip the '/' separator */
+
+ /* Parse localTransactionId (the unsigned integer after '/') */
+ errno = 0;
+ xid = strtoul(ptr, &endptr, 10);
+ if (endptr == ptr || errno != 0 || xid > UINT_MAX)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("invalid snapshot data in file \"%s\"", filename)));
+ vxid->localTransactionId = (LocalTransactionId) xid;
+ 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], [email protected]
Subject: Re: Cleanup: Replace sscanf with strtol/strtoul in snapmgr
In-Reply-To: <CAAJ_b974KcAQ=XMBfw-iy2tnbwNk9YnSA5XcQctf7N0GUSDRQA@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