Brecht Van Lommel (brecht)
- Group Reviewers
- rBSf8cd3d974daf: Code cleanup: add some asserts and fix a typo in BVH build.
rC8e2695a80399: Code cleanup: add some asserts and fix a typo in BVH build.
rBACf8cd3d974daf: Code cleanup: add some asserts and fix a typo in BVH build.
rBf8cd3d974daf: Code cleanup: add some asserts and fix a typo in BVH build.
Thanks. Can you explain the purpose of the NULL pointer asserts? I don't understand it.
|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.
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.