[PATCH weston-ivi-shell 04/15] ivi-shell supports a type of shell for In-Vehicle Infotainment system.

[Bill Spitzak]

Pekka Paalanen wrote:

>>>> +    wl_list_for_each(seat, &ec->seat_list, link) {
>>>> +        if (seat) {
>>>> +            shell->seat = seat;
>>> 'seat' cannot be NULL here. It seems you are assuming that there can be
>>> only one seat? Why?
>>>
>> Ideally, I agree. However I am in automotive software, and I have habit 
>> to avoid NULL access as much as possible in case of the worst scenario.
> 
> Then put an assert(), though in this case even that would be too much.
> You can never get a NULL from wl_list_for_each(). You might get a bad
> pointer if the list is corrupted, but the odds of getting exactly NULL
> are diminishing even with corruption.
> 
> Silently ignoring NULLs where they should never appear may hide real
> problems in testing, and even in production you'd probably want to log
> it somehow to help post mortem.

In addition, seeing an "if (x)" is a strong indication to the programmer 
that x *can* be NULL, and they will then make modifications to perceived 
bugs on the assumption that NULL is a possible value. This often leads 
to this same mistake being propagated to every function called by this 
and every use of a variable this value is stored into.

An assert(x) however is a strong indication that x is *not* NULL.

If you really are tempted to make a test so that if you missed a bug 
there is less chance of the optimized program crashing, what I do is 
something like this:

	assert(x);
	if (!x) return; // THIS SHOULD NOT HAPPEN