[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