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

Started by jinbingeabout 1 month ago7 messageshackers
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:

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/