Re: Change atoi to strtol in same place

On Tue, 8 Oct 2019 at 19:46, Joe Nelson <joe(at)begriffs(dot)com> wrote:
>
> David Rowley wrote:
> > The translation string here must be consistent over all platforms. I
> > think this will cause issues if the translation string uses %ld and
> > the platform requires %lld?
>
> A very good and subtle point. I'll change it to %lld so that a single
> format string will work everywhere.

The way to do this is to make a temp buffer and snprintf into that
buffer then use %s.

See basebackup.c where it does:

char buf[64];

snprintf(buf, sizeof(buf), INT64_FORMAT, total_checksum_failures);

ereport(WARNING,
(errmsg("%s total checksum verification failures", buf)));

as an example.

> > I think you should maybe aim for 2 patches here.
> >
> > Patch 1: ...
> >
> > Patch 2: Add additional validation where we don't currently do
> > anything. e.g pg_dump -j
> >
> > We can then see if there's any merit in patch 2 of if it's adding more
> > complexity than is really needed.
>
> Are you saying that my current patch adds extra constraints for
> pg_dump's -j argument, or that in the future we could do that? Because I
> don't believe the current patch adds any appreciable complexity for that
> particular argument, other than ensuring the value is positive, which
> doesn't seem too contentious.

> Maybe we can see whether we can reach consensus on the current
> parse-and-check combo patch, and if discussion drags on much longer then
> try to split it up?

I just think you're more likely to get a committer onside if you made
it so they didn't have to consider if throwing errors where we
previously didn't would be a bad thing. It's quite common to get core
infrastructure in first then followup with code that uses it. This
would be core infrastructure plus some less controversial usages of
it, then follow up with more. This was really just a suggestion. I
didn't dig into the patch in enough detail to decide on how many
places could raise an error that would have silently just have done
something unexpected beforehand.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services