[libdispatch-dev] [PATCH 12/17] extract source-type-specific initialization to per-type functions

Kevin Van Vechten kvv at apple.com
Sun Oct 18 23:34:56 PDT 2009


Neat.  Having an initializer makes sense and I suspect it will be used more extensively in the future as additional types are supported.

Though I would suggest that attributes such as ds_needs_rearm and ds_is_level be added to the dispatch_source_type_s structure and set in a generic fashion for all source types.  That might let the kevent-specific EV_CLEAR test for ds_is_adder be abstracted away as well.  (I think we had talked about making these data driven at some point but decided against making the cosmetic change late in the Snow Leopard release cycle.)

Thoughts?

Thanks!

Kevin

On Oct 18, 2009, at 8:04 AM, Paolo Bonzini wrote:

> As a first step towards eliminating struct kevent from source.c,
> some of the initialization is moved to separate functions.  This
> introduces a per-source-type function pointer that is called when
> creating a source.
> 
> The code is not 100% equal, but the only difference is that an
> assertion is delayed to after the full initialization---which is
> arguably better.
> ---
> src/source.c |   63 +++++++++++++++++++++++++++++++++++++++------------------
> 1 files changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/src/source.c b/src/source.c
> index b4f769d..d41bbda 100644
> --- a/src/source.c
> +++ b/src/source.c
> @@ -925,13 +925,29 @@ dispatch_source_attr_copy(dispatch_source_attr_t proto)
> struct dispatch_source_type_s {
> 	struct kevent ke;
> 	uint64_t mask;
> +	void (*init) (dispatch_source_t ds,
> +		      dispatch_source_type_t type,
> +		      uintptr_t handle,
> +		      unsigned long mask,
> +		      dispatch_queue_t q);
> };
> 
> +static void dispatch_source_type_timer_init (dispatch_source_t ds,
> +					     dispatch_source_type_t type,
> +					     uintptr_t handle,
> +					     unsigned long mask,
> +					     dispatch_queue_t q)
> +{
> +	ds->ds_needs_rearm = true;
> +	ds->ds_timer.flags = mask;
> +}
> +
> const struct dispatch_source_type_s _dispatch_source_type_timer = {
> 	.ke = {
> 		.filter = DISPATCH_EVFILT_TIMER,
> 	},
> 	.mask = DISPATCH_TIMER_INTERVAL|DISPATCH_TIMER_ONESHOT|DISPATCH_TIMER_ABSOLUTE|DISPATCH_TIMER_WALL_CLOCK,
> +	.init = dispatch_source_type_timer_init
> };
> 
> const struct dispatch_source_type_s _dispatch_source_type_read = {
> @@ -999,6 +1015,18 @@ const struct dispatch_source_type_s _dispatch_source_type_vfs = {
> };
> 
> #ifdef HAVE_MACH
> +
> +static void dispatch_source_type_mach_send_init (dispatch_source_t ds,
> +					     	 dispatch_source_type_t type,
> +					         uintptr_t handle,
> +						 unsigned long mask,
> +						 dispatch_queue_t q)
> +{
> +	static dispatch_once_t pred;
> +	ds->ds_is_level = false;
> +	dispatch_once_f(&pred, NULL, _dispatch_mach_notify_source_init);
> +}
> +
> const struct dispatch_source_type_s _dispatch_source_type_mach_send = {
> 	.ke = {
> 		.filter = EVFILT_MACHPORT,
> @@ -1006,14 +1034,25 @@ const struct dispatch_source_type_s _dispatch_source_type_mach_send = {
> 		.fflags = DISPATCH_MACHPORT_DEAD,
> 	},
> 	.mask = DISPATCH_MACH_SEND_DEAD,
> +	.init = dispatch_source_type_mach_send_init
> };
> 
> +static void dispatch_source_type_mach_recv_init (dispatch_source_t ds,
> +					     	 dispatch_source_type_t type,
> +					         uintptr_t handle,
> +						 unsigned long mask,
> +						 dispatch_queue_t q)
> +{
> +	ds->ds_is_level = false;
> +}
> +
> const struct dispatch_source_type_s _dispatch_source_type_mach_recv = {
> 	.ke = {
> 		.filter = EVFILT_MACHPORT,
> 		.flags = EV_DISPATCH,
> 		.fflags = DISPATCH_MACHPORT_RECV,
> 	},
> +	.init = dispatch_source_type_mach_recv_init
> };
> #endif
> 
> @@ -1097,38 +1136,22 @@ dispatch_source_create(dispatch_source_type_t type,
> 	ds->ds_dkev = dk;
> 	ds->ds_pending_data_mask = dk->dk_kevent.fflags;
> 	if ((EV_DISPATCH|EV_ONESHOT) & proto_kev->flags) {
> -#ifdef HAVE_MACH
> -		if (proto_kev->filter != EVFILT_MACHPORT) {
> -			ds->ds_is_level = true;
> -		}
> -#endif
> +		ds->ds_is_level = true;
> 		ds->ds_needs_rearm = true;
> 	} else if (!(EV_CLEAR & proto_kev->flags)) {
> 		// we cheat and use EV_CLEAR to mean a "flag thingy"
> 		ds->ds_is_adder = true;
> 	}
> 	
> -	// If its a timer source, it needs to be re-armed
> -	if (type->ke.filter == DISPATCH_EVFILT_TIMER) {
> -		ds->ds_needs_rearm = true;
> +	// Some sources require special processing
> +	if (type->init) {
> +		type->init (ds, type, handle, mask, q);
> 	}
> -	
> 	dispatch_assert(!(ds->ds_is_level && ds->ds_is_adder));
> #if DISPATCH_DEBUG
> 	dispatch_debug(ds, __FUNCTION__);
> #endif
> 
> -	// Some sources require special processing
> -#ifdef HAVE_MACH
> -	if (type == DISPATCH_SOURCE_TYPE_MACH_SEND) {
> -		static dispatch_once_t pred;
> -		dispatch_once_f(&pred, NULL, _dispatch_mach_notify_source_init);
> -	} else
> -#endif
> -	if (type == DISPATCH_SOURCE_TYPE_TIMER) {
> -		ds->ds_timer.flags = mask;
> -	}
> -
> 	_dispatch_retain(ds->do_targetq);
> 	return ds;
> 	
> -- 
> 1.6.2.5
> 
> 
> _______________________________________________
> libdispatch-dev mailing list
> libdispatch-dev at lists.macosforge.org
> http://lists.macosforge.org/mailman/listinfo.cgi/libdispatch-dev




More information about the libdispatch-dev mailing list