Re: Better detail logging for password auth failures

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2015-12-29 11:07:26 -0500, Tom Lane wrote:
>> In passing, the patch gets rid of a vestigial CHECK_FOR_INTERRUPTS()
>> call; it was added by e710b65c and IMO should have been removed again
>> by 6647248e. There's certainly no very good reason to have one right
>> at that spot anymore.

> Why? Doesn't seem like the worst place for an explicit interrupt check?

The only reason there was one there at all was that e710b65c added
code like this:

+ /*
+ * Disable immediate interrupts while doing database access. (Note
+ * we don't bother to turn this back on if we hit one of the failure
+ * conditions, since we can expect we'll just exit right away anyway.)
+ */
+ ImmediateInterruptOK = false;

... some catalog access here ...

+ /* Re-enable immediate response to SIGTERM/SIGINT/timeout interrupts */
+ ImmediateInterruptOK = true;
+ /* And don't forget to detect one that already arrived */
+ CHECK_FOR_INTERRUPTS();

In 6647248e you got rid of nine of these ten lines, leaving something
that's both pointless and undocumented. There are more than enough
CHECK_FOR_INTERRUPTS calls already in the auth code; there's not a
reason to expend code space on one here. (If MD5 ran long enough to
be worth interrupting, there would be an argument for a check inside
its hashing loop, but that still wouldn't be this check.)

regards, tom lane