Re: Better detail logging for password auth failures

<br /><br /><div class="gmail_quote">Em 31 de dezembro de 2015 04:56:55 BRST, Noah Misch <noah@leadboat.com>
escreveu:<blockquoteclass="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204);
padding-left:1ex;"><pre class="k9mail">On Wed, Dec 30, 2015 at 10:18:35AM -0500, Tom Lane wrote:<br /><blockquote
class="gmail_quote"style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> Andres Freund
<andres@anarazel.de>writes:<br /><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left:
1pxsolid #ad7fa8; padding-left: 1ex;"> On 2015-12-29 11:07:26 -0500, Tom Lane wrote:<br /><blockquote
class="gmail_quote"style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"> In passing,
thepatch gets rid of a vestigial CHECK_FOR_INTERRUPTS()<br /> call; it was added by e710b65c and IMO should have been
removedagain<br /> by 6647248e.  There's certainly no very good reason to have one right<br /> at that spot anymore.<br
/></blockquote></blockquote><br /><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px
solid#ad7fa8; padding-left: 1ex;"> Why? Doesn't seem like the worst place for an
 
explicit interrupt check?<br /> I think we don't really have a problem with too many such checks... We<br /> surely
couldmove it, but I don't really see how it's related to the<br /> topic at hand nor do I think it's really worth
ponderingabout<br /> extensively.<br /></blockquote></blockquote><br />Agreed.<br /><br /><blockquote
class="gmail_quote"style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> The only
reasonthere was one there at all was that e710b65c added<br /> code like this:<br /> <br /> +   /*<br /> +    * Disable
immediateinterrupts while doing database access.  (Note<br /> +    * we don't bother to turn this back on if we hit one
ofthe failure<br /> +    * conditions, since we can expect we'll just exit right away anyway.)<br /> +    */<br /> +
ImmediateInterruptOK= false;<br /> <br />     ... some catalog access here ...<br /> <br /> +   /* Re-enable immediate
responseto SIGTERM/SIGINT/timeout interrupts */<br /> +  
 
ImmediateInterruptOK = true;<br /> +   /* And don't forget to detect one that already arrived */<br /> +
CHECK_FOR_INTERRUPTS();<br/> <br /> In 6647248e you got rid of nine of these ten lines, leaving something<br /> that's
bothpointless and undocumented.  There are more than enough<br /> CHECK_FOR_INTERRUPTS calls already in the auth code;
there'snot a<br /> reason to expend code space on one here.  (If MD5 ran long enough to<br /> be worth interrupting,
therewould be an argument for a check inside<br /> its hashing loop, but that still wouldn't be this check.)<br
/></blockquote><br/>I see no general benefit from being parsimonious with CHECK_FOR_INTERRUPTS<br />calls or
documentingthem.  As you explain, it's probably fine to remove the<br />two calls that commit e710b65 had added.
However,the sole connection to<br />$SUBJECT is one of those two calls sharing a screenful with lines $SUBJECT<br
/>changed. The removal, if worthwhile, is worth a freestanding patch.<br />Squashing the changes makes both topics
harderto review.<br /><br />nm<br /><br /></pre></blockquote></div><br /> -- <br /> Sent from my Android device with
K-9Mail. Please excuse my brevity.