Alexey Bashtanov <bashtanov@imap.cc> writes:
> Current code looks somewhat fragile to me, as it takes some time to assure
> it doesn't pass already nulled out minv or maxv to atoi or strcmp again.
> I've rewritten it in a bit more clear way, so each comparison and
> nulling out happens only once.
Good idea ...
> Choosing between atol/atoll vs snprintf+strcmp,
> I preferred the former, as the latter is slower and uglier to me.
... but I was not thrilled with the way you did that. I don't like
having to back-patch new portability requirements with no time for them
to go through beta testing. What's more, this is randomly unlike the way
we're doing things elsewhere. I call your attention to pg_strtouint64
in src/backend/utils/adt/numutils.c.
What I did for the moment was to implement the comparison using
snprintf+strcmp, since that seemed like a very safely back-patchable
approach. While there's nothing particularly wrong with leaving
it like that, we could imagine creating a src/port module that
contains pg_strtouint64 and a corresponding implementation of
pg_strtoint64, and then fixing pg_dump to use pg_strtoint64 here.
I'd only consider doing that in HEAD though.
> BTW what should I do when submitting a patch if my autoconf adds
> some irrelevant changes?
The easy way is just to omit the configure file from the submitted
patch, noting that the committer must run autoconf. Most committers
probably would do that anyway (I certainly would).
However, if you want to do it right, or if you're someday given a
commit bit and you have to do it right, our project policy is that
we use exactly the GNU-released autoconf of the appropriate version,
so that anyone can reproduce the results, independently of patches
that their distribution might install into their platform's copy of
autoconf. So just pull the right autoconf tarball from any handy
GNU archive mirror, "configure --prefix=/something; make; make install",
prepend /something/bin to your PATH, and away you go. Personally
I keep autoconf N.NN in /usr/local/autoconf-N.NN/ so that I can use
whichever version is appropriate for the PG branch I'm working on.
regards, tom lane
