Re: Report from llvms static analysis tool

On 05.10.2015 18:28, Daniel wrote:
> > So if were to clear the scan-build warnings on that topic, mostly by
> > converting to assert, then when the dust settles we could perhaps mass
> > convert the remaining DBG_ASSERTs to SAL_WARN
>
> Is the conversion of DBG_ASSERTs to assert + SAL_WARN something we could
> add as an EasyHack? I also wouldn't mind adding this to the list of "long
> term fixes" I'm working on :-)

i'm afraid no.

the problem is, should a legacy assertion be converted to assert or to
SAL_WARN.  that is easy to answer in some cases (like when a pointer is
asserted not to be null that is then unconditionally dereferenced), but
in general requires some thought about whether it is trying to detect an
invalid internal state, or just warn about a scenario that appeared
unlikely to the author but is supposed to work even if it hasn't been
tested - and that requires some experience.

since the assert will actually abort on failure, we can't add too many
of these in too short a time or developers will be overwhelmed by the
assertion failures and complain that they can't fix the bug that they
need to fix because of all the assertions getting in the way and stop
using debug builds.

also, QA will sometimes file bugs about assert failures, which is
actually very useful if there are sufficiently few such reports that
they can be fixed quickly.

of course we could convert everything automatically to SAL_WARN, but
leaving the legacy assertions has the advantage that it is obvious that
for those cases nobody has thought about whether it *should* be an
assert or not.