[PATCH 15/20] chainimpl.h: Add _Assert() to _Chain_Initialize_empty()

Peter Dufault dufault at hda.com
Wed Nov 26 16:29:26 UTC 2014


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.

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?".

- 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.

- 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.

> On Nov 25, 2014, at 18:02 , Joel Sherrill <Joel.Sherrill at 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




More information about the devel mailing list