Gregory Stark wrote:
> "Andrew Dunstan" <andrew@dunslane.net> 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