pg_resetwal.c: duplicate '0' in hex character set for -l option validation

Started by jinbinge7 months ago7 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t53435
psql -h localhost -U postgres

Built from patchset v3 (message #3), July 27, 2026 at 03:39 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t53435_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t53435_3 && git checkout t53435_3

Patchset v3 (message #3) is on t53435_3

Jump to latest
#1jinbinge
jinbinge@126.com

Hi,

In pg_resetwal.c, around line 310, there's a minor redundancy in the character set string for validating the -l option argument.

The current code is:

case 'l':
if (strspn(optarg, "01234567890ABCDEFabcdef") != XLOG_FNAME_LEN)
{
pg_log_error("invalid argument for option %s", "-l");
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
exit(1);
}

The string "01234567890ABCDEFabcdef" contains a duplicated '0' in the digit portion ("01234567890" instead of "0123456789").

While this redundancy doesn't affect functionality (the allowed character set remains unchanged and still properly validates

hexadecimal digits 0-9, A-F, a-f), it could cause confusion for code readers.

For improved clarity and to follow conventional hexadecimal representation, I suggest using the standard character set:

"0123456789ABCDEFabcdef". This change would be purely cosmetic with no behavioral impact, but would eliminate the unnecessary duplication.

Thanks for considering this minor improvement.

Regards,
Jinbinge

Attachments:

v1-0001-Fix-duplicate-0-in-hex-character-set-for-l-option.patchapplication/octet-stream; name=v1-0001-Fix-duplicate-0-in-hex-character-set-for-l-option.patch; x-cm-securityLevel=0Download+1-2
#2Michael Paquier
michael@paquier.xyz
In reply to: jinbinge (#1)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

On Mon, Mar 02, 2026 at 10:13:38AM +0800, jinbinge wrote:

"0123456789ABCDEFabcdef". This change would be purely cosmetic with
no behavioral impact, but would eliminate the unnecessary
duplication.

That's entirely cosmetic, well why not.. There is also a
01234567890ABCDEF in src/bin/pg_upgrade/controldata.c. Perhaps we
could sanitize the whole as of 0123456789ABCDEF, which is larger in
trend compared to 1234567890ABCDEF when it comes to the C code, not
the tests.
--
Michael

#3jinbinge
jinbinge@126.com
In reply to: Michael Paquier (#2)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

At 2026-03-02 10:24:52, "Michael Paquier" <michael@paquier.xyz> wrote:

On Mon, Mar 02, 2026 at 10:13:38AM +0800, jinbinge wrote:

"0123456789ABCDEFabcdef". This change would be purely cosmetic with
no behavioral impact, but would eliminate the unnecessary
duplication.

That's entirely cosmetic, well why not.. There is also a
01234567890ABCDEF in src/bin/pg_upgrade/controldata.c. Perhaps we
could sanitize the whole as of 0123456789ABCDEF, which is larger in
trend compared to 1234567890ABCDEF when it comes to the C code, not
the tests.
--

Michael

Thank you for the feedback. I have also fixed the same redundant hex character string
in src/bin/pg_upgrade/controldata.c.

After searching for "01234567890" and "01234567890ABCDEF" (excluding test files),
no other occurrences were found.
The attached v2 patch replaces non‑standard or redundant hexadecimal digit strings
in C source files with the conventional "0123456789ABCDEF".

--
Jinbinge

Attachments:

t53435_3
v2-0001-Standardize-hex-digit-character-strings.patchapplication/octet-stream; name=v2-0001-Standardize-hex-digit-character-strings.patch; x-cm-securityLevel=0Download+2-3
#4Michael Paquier
michael@paquier.xyz
In reply to: jinbinge (#3)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

On Mon, Mar 02, 2026 at 11:38:30AM +0800, jinbinge wrote:

After searching for "01234567890" and "01234567890ABCDEF" (excluding test files),
no other occurrences were found.
The attached v2 patch replaces non‑standard or redundant hexadecimal digit strings
in C source files with the conventional "0123456789ABCDEF".

Sounds find to me. As this is purely cosmetic, I am adding it into my
next batch of cosmetic changes merged together.
--
Michael

#5Chao Li
li.evan.chao@gmail.com
In reply to: Michael Paquier (#4)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

On Mar 4, 2026, at 08:31, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Mar 02, 2026 at 11:38:30AM +0800, jinbinge wrote:

After searching for "01234567890" and "01234567890ABCDEF" (excluding test files),
no other occurrences were found.
The attached v2 patch replaces non‑standard or redundant hexadecimal digit strings
in C source files with the conventional "0123456789ABCDEF".

Sounds find to me. As this is purely cosmetic, I am adding it into my
next batch of cosmetic changes merged together.
--
Michael

Hi Michael,

In your recent batch commit 462fe0ff6215ae222fc866af621e4b86462289b1, I noticed that nobody was credited. Does this mean that people who report typo-like problems will no longer be credited?

From my experience, submitting trivial patches is often an easy entry point for new hackers to get familiar with the patch process. For that reason, I wonder if it would make sense to at least credit them as reporters?

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

#6Michael Paquier
michael@paquier.xyz
In reply to: Chao Li (#5)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

On Wed, Mar 04, 2026 at 09:16:12AM +0800, Chao Li wrote:

In your recent batch commit
462fe0ff6215ae222fc866af621e4b86462289b1, I noticed that nobody was
credited.

Please note that I have discussed this question with others offline,
hence the delay in my reply.

Does this mean that people who report typo-like problems
will no longer be credited?

Yes, that was on purpose. We have seen a large increase of these
trivial patches to pgsql-hackers lately for some reason. Maybe it is
LLMs or scanners or just a lot of new people with fresh eyes looking
at the code. It started to get tedious to process them one by one,
and we discussed among committers if we could have a more lightweight
process for them. As an experiment, I started to collect all such
changes into these larger batches. Not crediting the reporters is
part of the experiment; tracking credit for trivial typo fixes is
hardly worth the effort, skipping that makes the process less
time-consuming.
--
Michael

#7Chao Li
li.evan.chao@gmail.com
In reply to: Michael Paquier (#6)
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation

On Mar 5, 2026, at 07:23, Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Mar 04, 2026 at 09:16:12AM +0800, Chao Li wrote:

In your recent batch commit
462fe0ff6215ae222fc866af621e4b86462289b1, I noticed that nobody was
credited.

Please note that I have discussed this question with others offline,
hence the delay in my reply.

Does this mean that people who report typo-like problems
will no longer be credited?

Yes, that was on purpose. We have seen a large increase of these
trivial patches to pgsql-hackers lately for some reason. Maybe it is
LLMs or scanners or just a lot of new people with fresh eyes looking
at the code. It started to get tedious to process them one by one,
and we discussed among committers if we could have a more lightweight
process for them. As an experiment, I started to collect all such
changes into these larger batches. Not crediting the reporters is
part of the experiment; tracking credit for trivial typo fixes is
hardly worth the effort, skipping that makes the process less
time-consuming.
--
Michael

Thanks for the detailed explanation. I asked because I will have a 25-min talk in the coming PGConf.dev about the hacker experience, so I wanted to confirm this change and mention that in my talk.

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