[PATCH wayland 4/7] Add support for custom dispatchers

Bill Spitzak spitzak at gmail.com
Mon Feb 18 09:07:40 PST 2013


On 02/18/2013 08:03 AM, Jason Ekstrand wrote:

> +	if (dispatcher == NULL) {
> +		wl_interface_default_dispatcher(target, opcode,
> +						closure->message,
> +						data, closure->args);
> +	} else {
> +		(*dispatcher)(target, opcode, closure->message,
> +			      data, closure->args);
> +	}

I don't know if this is covered in any guidelines for Wayland, but I 
don't like writing the negative test like this. It seems too easy to 
accidentally read the first case as that not-zero one.

The politically-correct solution is to write it like this by swapping 
the if statements:

    if (dispatcher != NULL) {
       (*dispatcher)(...);
    } else {
       default_dispatcher(...);
    }

Personally I prefer this, IMHO this makes it much more obvious what is 
being tested and no chance of reading it the wrong way:

    if (dispatcher) {
       ...

Though I think there may be disagreement about this, so official Wayland 
policy should be followed.

It may also make sense to test for zero in the api that sets dispatcher 
and change it to default there, rather than testing on every call. That 
would also make it clear that the api for the default and user-settable 
dispatchers is the same.



More information about the wayland-devel mailing list