[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