Re: refactoring - standardize integer parsing in front-end utilities - Mailing list pgsql-hackers

From Joe Nelson
Subject Re: refactoring - standardize integer parsing in front-end utilities
Date
Msg-id 20200112214338.dnmeacld5bwyr3fu@begriffs.com
Whole thread Raw
In response to Re: Change atoi to strtol in same place  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
Joe Nelson wrote:
> > I see this patch has been moved to the next commitfest. What's the next
> > step, does it need another review?

Peter Eisentraut wrote:
> I think you need to promote your patch better.

Thanks for taking the time to revive this thread.

Quick sales pitch for the patch:

* Standardizes bounds checking and error message format in utilities
  pg_standby, pg_basebackup, pg_receivewal, pg_recvlogical, pg_ctl,
  pg_dump, pg_restore, pg_upgrade, pgbench, reindexdb, and vacuumdb
* Removes Undefined Behavior caused by atoi() as described
  in the C99 standard, section 7.20.1
* Unifies integer parsing between the front- and back-end using
  functions provided in https://commitfest.postgresql.org/25/2272/

In reality I doubt my patch is fixing any pressing problem, it's just a
small refactor.

> The thread subject and the commit fest entry title are somewhat
> nonsensical and no longer match what the patch actually does.

I thought changing the subject line might be discouraged, but since you
suggest doing it, I just did. Updated the title of the commitfest entry
https://commitfest.postgresql.org/26/2197/ as well.

> The patch contains no commit message

Does this list not accept plain patches, compatible with git-apply?
(Maybe your point is that I should make it as easy for committers as
possible, and asking them to invent a commit message is just extra
work.)

> and no documentation or test changes

The interfaces of the utilities remain the same. Same flags. The only
change would be the error messages produced for incorrect values.

The tests I ran passed successfully, but perhaps there were others I
didn't try running and should have.

> Moreover, a lot of this error message tweaking is opinion-based, so
> it's more burdensome to do an objective review.  This patch is
> competing for attention against more than 200 other patches that have
> more going for them in these aspects.

True. I think the code looks nicer and the errors are more informative,
but I'll leave it in the community's hands to determine if this is
something they want.

Once again, I appreciate your taking the time to help me with this
process.



pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: Consolidate 'unique array values' logic into a reusable function?
Next
From: Daniel Gustafsson
Date:
Subject: Question regarding heap_multi_insert documentation