[PATCH] Add Solaris ports as a tevent backend.

David Disseldorp ddiss at suse.de
Thu Feb 12 11:05:32 MST 2015


Hi Jeremy,

The patch looks good, but I unfortunately don't have a Solaris system to
test against. A few comments below, which I'd like to see addressed
before pushing...

On Tue, 10 Feb 2015 14:59:50 -0800, Jeremy Allison wrote:

...
> --- a/lib/replace/system/select.h
> +++ b/lib/replace/system/select.h
> @@ -34,6 +34,10 @@
>  #include <sys/epoll.h>
>  #endif
>  
> +#ifdef HAVE_PORT_H
> +#include <port.h>
> +#endif

Should probably use HAVE_SOLARIS_PORTS here.

...
> --- a/lib/tevent/tevent.c
> +++ b/lib/tevent/tevent.c
> @@ -126,6 +126,10 @@ static void tevent_backend_init(void)
>  #ifdef HAVE_EPOLL
>  	tevent_epoll_init();
>  #endif
> +#ifdef HAVE_SOLARIS_PORTS
> +	tevent_port_init();
> +#endif
> +
>  	tevent_standard_init();
>  }
>  
> @@ -139,7 +143,11 @@ _PRIVATE_ const struct tevent_ops *tevent_find_ops_byname(const char *name)
>  		name = tevent_default_backend;
>  	}
>  	if (name == NULL) {
> +#ifdef HAVE_SOLARIS_PORTS
> +		name = "port";
> +#else
>  		name = "standard";
> +#endif

tevent_port_init() could call tevent_set_default_backend() to avoid this
hunk.

...

> --- /dev/null
> +++ b/lib/tevent/tevent_port.c
> @@ -0,0 +1,767 @@

...

> +static bool store_port_association(struct port_event_context *port_ev,
> +				struct tevent_fd *fde,
> +				int events)
> +{
> +	struct port_associate_vals *val;
> +
> +	for (val = port_ev->po_vals; val; val = val->next) {
> +		if (val->fde->fd == fde->fd) {
> +			/* Association already attached to fd. */
> +			if (val->events != events) {
> +				val->events = events;
> +				val->associated_event = false;
> +			}
> +			return true;
> +		}
> +	}
> +
> +	val = talloc_zero(port_ev, struct port_associate_vals);
> +	if (val == NULL) {
> +		return false;
> +	}
> +
> +	val->port_ev = port_ev;
> +	val->fde = fde;
> +	val->events = events;
> +	val->associated_event = false;
> +
> +	DLIST_ADD(port_ev->po_vals, val);
> +	talloc_set_destructor(val, port_associate_vals_destructor);
> +
> +	return true;
> +}

As the port_association is per-fde, wouldn't it be simpler to store it
directly in fde->additional_data, alongside any multiplexed-fde? That
way you'd avoid the lookup on store and delete, and would only need
to walk the ev->fd_events list in associate_all_events().

...

> +static struct tevent_fd *port_event_add_fd(struct tevent_context *ev, TALLOC_CTX *mem_ctx,
> +					    int fd, uint16_t flags,
> +					    tevent_fd_handler_t handler,
> +					    void *private_data,
> +					    const char *handler_name,
> +					    const char *location)
> +{
> +	struct port_event_context *port_ev = talloc_get_type(ev->additional_data,
> +							   struct port_event_context);

tevent_epoll was recently changed to use talloc_get_type_abort() here,
please do the same.

...

> +static void port_event_set_fd_flags(struct tevent_fd *fde, uint16_t flags)
> +{
> +	struct tevent_context *ev;
> +	struct port_event_context *port_ev;
> +
> +	if (fde->flags == flags) {
> +		return;
> +	}
> +
> +	ev = fde->event_ctx;
> +	port_ev = talloc_get_type(ev->additional_data, struct port_event_context);

talloc_get_type_abort() here too.



More information about the samba-technical mailing list