On Wed, Nov 26, 2014 at 11:29 AM, Peter Dufault <dufa...@hda.com> wrote: > These are minor nits, but I'll bring them up anyway because as I review these > changes I keep thinking about them. > > If you have a small-codespace target that can togenerate faults on > low-address-space accesses then these NULL dereferences are going to be > caught in the exception handler and don't help. These assertions are adding > a negligible amount of code overhead to those small-codespace targets. > You can optimize the asserts out. It might be sensible to add a class of "ASSERT_NOT_NULL" in case one wants to keep other asserts but catch NULLs in the exception handler.
> These changes are losing information about why something is not NULL. If > you're inspecting the code and see the _Assert() you think "Why will my > system go into fail-safe-shutdown in this situation? Why is this known to be > not NULL?". > Great point. Asserts should have comments attached to them in general. > - Could an _Assert_not_NULL() macro be used to eliminate the negligible > overhead? It could be defined differently for targets that unmap the bottom > of memory. > Hey look, thinking the same thing, I should finish reading before writing... > - Could an _Assert_known_not_NULL() macro be used for pointers that are > absolutely, positively known to be not-NULL but somehow they may be > corrupted? That preserves information for someone looking at the code in the > future. Instead of a check-in comment that says "This can't be NULL, it was > checked before so we can assert this" the reviewer will know it is supposed > to have been previously checked to be not NULL, so either someone broke the > code or something corrupted memory. > What is the difference between the previous two variants? Would you expect to optimize out "Assert_known" in all production code? Great points, Gedare >> On Nov 25, 2014, at 18:02 , Joel Sherrill <joel.sherr...@oarcorp.com> wrote: >> >> CodeSonar flagged this as a potential NULL deference. That should never >> occur but adding the _Assert() ensures we are checking that. > > Peter > ----------------- > Peter Dufault > HD Associates, Inc. Software and System Engineering > > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
