Gregory Stark wrote:
"Andrew Dunstan" <[EMAIL PROTECTED]> writes:

Gregory Stark wrote:
Attached is an updated patch.

This patch appears to add a nonexistent test to the regression schedules.

I must have forgotten to cvs add it. Sorry.

Also, I forgot to mention previously there is an unrelated trivial hunk in
here. I noticed we free the password early, presumably for security reasons,
but don't actually overwrite the memory at that point. I added a memset in
there, otherwise the free seems kind of pointless. I normally don't bother
with "security" features like that since they don't really add any security
but as long as it's there it may as well do something vaguely useful.

Greg,

In general this looks quite reasonable. It's a very large patch for a feature of this size, but luckily it's mostly changes due to the new pset structure rather than new code.

There are some places that it doesn't use PG style (e.g. no newline before brace after if) and some comments that need to be fixed (e.g. /* XXX get result */ )

If you can fix that I'll apply the patch - I especially want to get it tested widely via the buildfarm. I am leaving town for about 5 or 6 days on Friday morning, and my availability will be somewhat restricted during that time, so if this isn't fixed pronto it will possibly have to wait till my return or till another reviewer takes pity on you :-)

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

              http://www.postgresql.org/docs/faq

Reply via email to