Em 31 de dezembro de 2015 04:56:55 BRST, Noah Misch <n...@leadboat.com> escreveu: >On Wed, Dec 30, 2015 at 10:18:35AM -0500, Tom Lane wrote: >> Andres Freund <and...@anarazel.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? >> > I think we don't really have a problem with too many such checks... >We >> > surely could move it, but I don't really see how it's related to >the >> > topic at hand nor do I think it's really worth pondering about >> > extensively. > >Agreed. > >> 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.) > >I see no general benefit from being parsimonious with >CHECK_FOR_INTERRUPTS >calls or documenting them. As you explain, it's probably fine to >remove the >two calls that commit e710b65 had added. However, the sole connection >to >$SUBJECT is one of those two calls sharing a screenful with lines >$SUBJECT >changed. The removal, if worthwhile, is worth a freestanding patch. >Squashing the changes makes both topics harder to review. > >nm > > >-- >Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >To make changes to your subscription: >http://www.postgresql.org/mailpref/pgsql-hackers
-- Sent from my Android device with K-9 Mail. Please excuse my brevity.
