Re: Tighten up range checks for pg_resetwal arguments

Hi Heikki,

This patch looks like a straightforward change, but I see a correctness issue and a few small comments.

> On Dec 4, 2025, at 03:07, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> While working on the 64-bit multixid offsets patch and commit 94939c5f3a, I got a little annoyed by how lax
pg_resetwalis about out-of-range values. These are currently accepted, for example: 
>
> # Negative XID
> pg_resetwal -D data -x -1000
>
> # XID larger than 2^32   (on some platforms)
> pg_resetwal -D data -x 10000000000
>
> The first attached patch tightens up the parsing to reject those.
>
> The second attached patch is just refactoring. Currently, we use invalid values for the variables backing each of the
optionsto mean "option was not given". I think it would be more clear to have separate boolean variables for that. I
didthat for the --multixact-ids option in commit f99e30149f already, because there was no unused value for multixid
thatwe could use. This patch expands that to all the options. 
>
> - Heikki
>
<0001-pg_resetwal-Reject-negative-and-out-of-range-argumen.patch><0002-pg_resetwal-Use-separate-flags-for-whether-an-option.patch>

1 - 0002 - correctness issue
```
-    if (set_oid != 0)
-        ControlFile.checkPointCopy.nextOid = set_oid;
+    if (next_oid_given)
+        ControlFile.checkPointCopy.nextOid = next_oid_val;
```

As OID 0 is invalid, the old code checks that. But the new code checks only next_oid_given, which loses the validation
ofinvalid OID 0. 

This issue applies to multiple parameters.

2 - 0001
```
+/*
+ * strtouint32_strict -- like strtoul(), but returns uint32 and doesn't accept
+ * negative values
+ */
+static uint32
+strtouint32_strict(const char *restrict s, char **restrict endptr, int base)
+{
+    unsigned long val;
+    bool        is_neg;
+
+    /* skip leading whitespace */
+    while (isspace(*s))
+        s++;
+
+    /*
+     * Is it negative?  We still call strtoul() if it was, to set 'endptr'.
+     * (The current callers don't care though.)
+     */
+    is_neg = (*s == '-');
+
+    val = strtoul(s, endptr, base);
+
+    /* reject if it was negative */
+    if (errno == 0 && is_neg)
+    {
+        errno = ERANGE;
+        val = 0;
+    }
+
+    /*
+     * reject values larger than UINT32_MAX on platforms where long is 64 bits
+     * wide.
+     */
+    if (errno == 0 && val != (uint32) val)
+    {
+        errno = ERANGE;
+        val = UINT32_MAX;
+    }
+
+    return (uint32) val;
+}
```

I guess this function doesn’t have to check “-“ by itself, it leads some edge-case not to be well handled, for example
“-0”is still 0, not a negative value. We can use strtoll() convert input string to a singed long long, and check if
valueis negative. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/