[PATCH V2] event: assert wl_event_source pointer is NULL.

[Bryce W. Harrington]

On Mon, May 12, 2014 at 11:26:01AM +0530, Srivardhan Hebbar wrote:
> Signed-off-by: Srivardhan Hebbar <sri.hebbar at samsung.com>
> ---
>  src/event-loop.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/event-loop.c b/src/event-loop.c
> index 9790cde..57e3fed 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -312,7 +312,11 @@ wl_event_source_check(struct wl_event_source *source)
>  WL_EXPORT int
>  wl_event_source_remove(struct wl_event_source *source)
>  {
> -	struct wl_event_loop *loop = source->loop;
> +	struct wl_event_loop *loop;
> +
> +	assert(!source);

While I notice event-loop.c does include assert.h, there are no other
asserts in this file; most other calls are returning NULL or -1 on error
conditions, which I'm guessing might be better?  Especially for cleanup
routines, it's not uncommon to return success when the provided object
is already null or otherwise doesn't need cleanup.  Generally in library
you want to bubble errors up for the library user to decide how to
handle rather than exiting or asserting.

Also, looking through event-loop.c there's many other routines that take
a pointer argument to a struct and reference members without first
checking the pointer's validity.  So I'm gathering that this code
expects source's validity to be verified before calling into
wl_event_source_remove()?  In which case, there's no need for a NULL
pointer check.  (Or, conversely, all the incoming pointers in
event-loop.c need checked.)

I'd hazard a guess that the reason the functions in event-loop.c don't
validate their inputs is for performance reasons.

>  	/* We need to explicitly remove the fd, since closing the fd
>  	 * isn't enough in case we've dup'ed the fd. */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel