Re: Coverity Open Source Defect Scan of PostgreSQL

Tom Lane wrote:
> "Marc G. Fournier" <scrappy(at)postgresql(dot)org> 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.

Agreed. The idea of modifying our binary in any way to help a sanity
tool not complain is totally backwards.

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +