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(at)iki(dot)fi> wrote:
>
> While working on the 64-bit multixid offsets patch and commit 94939c5f3a, I got a little annoyed by how lax pg_resetwal is 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 options to mean "option was not given". I think it would be more clear to have separate boolean variables for that. I did that for the --multixact-ids option in commit f99e30149f already, because there was no unused value for multixid that we 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 of invalid 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 value is negative.

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