Page MenuHome

Misc asserts, typos, initializations, flagged by static code analysis
ClosedPublic

Authored by John Pavel (jrp) on Apr 18 2014, 12:55 AM.

Diff Detail

Event Timeline

Thanks. Can you explain the purpose of the NULL pointer asserts? I don't understand it.

intern/cycles/render/graph.cpp
507 ↗(On Diff #1491)

Can you explain the purpose of asserts like this? This pointer gets dereferenced on the next line, so it should of course not be NULL. But if we add asserts for that kind of case we'd have to add thousands all over the code.

If static code analysis reveals a bug here (is that the case?) we should fix it rather than add an assert.

Brecht Van Lommel (brecht) requested changes to this revision.Apr 18 2014, 12:17 PM
John Pavel (jrp) requested a review of this revision.Apr 18 2014, 12:33 PM

The asserts are where the static analysis finds a potential path through the code that could lead to the dereferencing of a NULL pointer.

In these cases, a value is sometimes explicitly initialized to NULL and then used later on.

There is most likely no current bug, but some future change could trigger the assert.

Ok, I will commit a few of the changes.

But I don't see much point in adding the NULL pointer asserts, as far as I can tell this is just a false positive by the static code analysis. I rather only use asserts for conditions that are not obviously true from a quick glance at the code or tricky to ensure in some way. About the stack space usage, I don't see that as a problem, it's not that much. The BVH binning change to MAX_BINS also seemed unnecessary to me.