Craig Ringer <cr...@2ndquadrant.com> writes:
> The attached patch series adds support for detecting coding errors where a
> stack-allocated ErrorContextCallback is not popped from the
> error_context_stack before the variable leaves scope.
So my immediate thoughts about this are
(1) It's mighty invasive for what it accomplishes. AFAIK we have
had few of this class of bug, and so I'm not excited about touching
every callback use in the tree to add defenses. It's also not great
that future code additions won't be protected unless they remember
to add these magic annotations. The PG_TRY application is better since
it's wrapped into the existing macro.
(2) I don't like that this is adding runtime overhead to try to detect
such bugs.
(3) I think it's a complete failure that such a bug will only be
detected if the faulty code path is actually taken. I think it's
quite likely that any such bugs that are lurking are in "can't
happen", or at least hard-to-hit, corner cases --- if they were in
routinely tested paths, we'd have noticed them by now. So it'd be far
more helpful if we had a static-analysis way to detect such issues.
Thinking about (3), I wonder whether there's a way to instruct Coverity
to detect such errors.
regards, tom lane
