Re: Change atoi to strtol in same place

Robert Haas wrote:
> > -1. I think it's very useful to have routines for this sort of thing
> > that return an error message rather than emitting an error report
> > directly. That gives the caller a lot more control.

Michael Paquier wrote:
> 1) Consistency with the error messages makes less work for translators,
> who already have a lot to deal with.

Agreed that the messages can be slightly inconsistent. I tried to make
the new messages match the styles of other messages in their respective
utilities. Maybe the bigger issue here is inconsistent output styles
across the utilities in general:

pg_standby.c includes flag names
%s: -r maxretries %s
pg_basebackup.c writes the settings out in words
invalid compression level: %s

Note that the final %s in those examples will expand to a more detailed
message. For example passing "-Z 10" to pg_dump in the current patch will
output:

pg_dump: error: invalid compression level: 10 is outside range 0..9

> 2) A centralized error message can provide the same level of details.

Even assuming we standardize the message format, different callers have
different means to handle the messages. The front-end utilities affected in my
patch use calls as varied as fprintf, pg_log_error, write_stderr and pg_fatal.
Thus pg_strtoint64_range needs more flexibility than calling pg_log_error
internally.

> 3) I think that we should not expose directly the status values of
> pg_strtoint_status in pg_strtoint64_range(), that's less for module
> authors to worry about, and that would be the same approach as we are
> using for the wrappers of pg_strto[u]intXX() in the patch of the other
> thread (see pg_strto[u]intXX_check for example in [1]).

The pg_strto[u]intXX_check functions can return the integer directly only
because they handle errors with ereport(ERROR, ...). However, as I mentioned
earlier, this is not always what the front-end utilities need to do.

--
Joe Nelson https://begriffs.com