[PATCH V2] event: assert wl_event_source pointer is NULL.
Bill Spitzak
spitzak at gmail.com
Mon May 12 12:31:44 PDT 2014
The reason functions should not validate pointers that are supposed to
be non-NULL is because it is misleading. An "if (x)..." in the code is a
very very strong indicator to a programmer that x can legitimately have
a NULL value. No amount of documentation is going to change that
programmer's mind, the source code overrules everything (besides they
probably did not read the documentation anyway).
Therefore such if statements should not be in there. It has nothing to
do with performance, it has all to do with making the source code clear
and not misleading.
An assert(x) is fine. That is not misleading, in fact it is a
confirmation that the pointer is not supposed to be NULL and thus a good
idea. If this really bugs you that it is not done by optimized code make
your own assert that runs even when optimized, it can even log the
problem or attempt to recover. The important thing is that the word
"assert" is in the name.
However in my personal experience it is better to crash there than to
crash at a totally unrelated location that was relying on the skipped
code being executed. It is way easier to debug, and I certainly have
made a first step of deleting all such tests in order to figure out a crash.
>> - struct wl_event_loop *loop = source->loop;
>> + struct wl_event_loop *loop;
>> +
>> + assert(!source);
Are you sure this assert is correct? Looks backwards.
More information about the wayland-devel
mailing list