On Thu, 9 Mar 2006, Tom Lane wrote:
"Marc G. Fournier" <[EMAIL PROTECTED]> writes:
Why? I don't think we are able to run 'embedded' now as it is, so its not
like we're dealign with system with small disk spaces :) how much bigger
would adding that exit() make the binary?
It's not only the exit(), as the elevel parameter isn't always a
constant. The proposed patch would at a minimum expose us to
double-evaluation risks. I kinda doubt there are any cases where an
elevel parameter expression has side-effects, so that objection may be
mostly hypothetical, but nonetheless we are talking about more than just
wasting a few bytes. It's not impossible that the patch would introduce
outright bugs. Consider something like
/* ENOENT is expected, anything else is not */
elog(errno == ENOENT ? DEBUG : ERROR, ...)
By the time control comes back from elog, errno would likely be
different, and so this would result in an unexpected exit() call
if the patch is in place. I'd be the first to call the above poor
coding, but it wouldn't be a bug ... unless the errno is rechecked.
It's been asserted that Coverity can be taught to understand about
elog/ereport without this sort of hack, so I'd rather take that tack.
I realize that this might sound 'odd' ... but, would it maybe make sense
to document the code around stuff like this as to why we do it the way we
do? Basically, we're debating how we could change the code to clean
things up for Coverity's analysis, and by the fact that we're getting both
sides of the discussion, there are ppl that think that the code
could/should be changed ... the arguments against make sense, but instead
of coming back to revisit this some time in the future, documenting it in
the code as to why we are doing it this way in the first place might save
time?
----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: [EMAIL PROTECTED] Yahoo!: yscrappy ICQ: 7615664
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
