Serguei Mokhov <mokhov@cs.concordia.ca> writes:
> Could you point out, what's exactly unclean? Most importantly,
> what would be the way you'd fix it?
What's bugging me is that even though the patch goes out of its way to
share code, there still seems to be a lot of duplicate code. You're not
getting the full benefit of sharing code between both cases, yet you
still seem to be paying the price of extra code complexity compared to
just copy-paste-and-modify.
What I'm thinking about is
-- pq_getstr takes a length limit parameter, which is (say) 0 for "no
limit". Since it's only called in one place, we can just change its
API; there's hardly any point in providing a backward-compatible routine.
(BTW, I agree with your implementation choice to check the limit only
once per bufferload, and thus have a fuzzy limit, but this needs to be
documented.)
-- pq_getstring becomes pq_getstring_bounded, with a limit parameter
that it just passes down.
-- We can "#define pq_getstring(buf) pq_getstring_bounded(buf, 0)" to
avoid changing the call sites that want unbounded input (not that there
are that many of 'em, but we may as well provide the macro).
Will adjust your patch in this way and apply.
regards, tom lane
