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

[Nobuhiko Tanibata]

2014-03-12 04:10 に 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

Hi pq and Bill,

Thank you a lot. I will follow the above way.

BR,
Nobuhiko