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

[Joel Sherrill]

On November 26, 2014 10:29:26 AM CST, Peter Dufault <dufault at 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.

These only generate code when the --enable-rtems-debug flag is specified. Otherwise no code is generated.

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

These are run-time fault issues for code built without debug assertions.

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