Re: Incorrect usage of strtol, atoi for non-numeric junk inputs

On Wed, Jul 14, 2021 at 11:02:47AM -0400, Alvaro Herrera wrote:
> On 2021-Jul-14, Kyotaro Horiguchi wrote:
>>> Should we take this occasion to reduce the burden of translators and
>>> reword that as "%s must be in range %d..%d"? That could be a separate
>>> patch.
>
> Yes, please, let's do it here.

Okay.

> No, this doesn't work. When the first word is something that is
> not to be translated (a literal parameter name), let's use a %s (but of
> course the values must be taken out of the phrase too). But when it is
> a translatable phrase, it must be present a full phrase, no
> substitution:
>
> pg_log_error("%s must be in range %s..%s", "extra_float_digits", "-15", "3");
> pg_log..("compression level must be in range %s..%s", "0", "9"))
>
> I think the purity test is whether you want to use a _() around the
> argument, then you have to include it into the original message.

After thinking about that, it seems to me that we don't have that much
context to lose once we build those error messages to show the option
name of the command. And the patch, as proposed, finishes with the
same error message patterns all over the place, which would be a
recipe for more inconsistencies in the future. I think that we should
centralize all that, say with a small-ish routine in a new file called
src/fe_utils/option_parsing.c that uses strtoint() as follows:
bool option_parse_int(const char *optarg,
const char *optname,
int min_range,
int max_range,
int *result);

Then this routine may print two types of errors through
pg_log_error():
- Incorrect range, using min_range/max_range.
- Junk input.
The boolean status is here to let the caller do any custom exit()
actions he wishes if there one of those two failures. pg_dump has
some of that with exit_nicely(), for one.
--
Michael