Обсуждение: pg_resetwal.c: duplicate '0' in hex character set for -l option validation
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
Jinbinge
Вложения
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation
От
Michael Paquier
Дата:
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
Вложения
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. >-->MichaelThank you for the feedback. I have also fixed the same redundant hex character stringin 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 stringsin C source files with the conventional "0123456789ABCDEF".--Jinbinge
Вложения
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation
От
Michael Paquier
Дата:
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
Вложения
> 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 meanthat 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 patchprocess. 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/
Re: pg_resetwal.c: duplicate '0' in hex character set for -l option validation
От
Michael Paquier
Дата:
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
Вложения
> 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 hackerexperience, 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/