On Wed, Jun 20, 2012 at 08:59:20AM -0500, Jeremy White wrote: > > I've started a conversation on the xorg-devel mailing list[1], and I'll > > follow up directly in irc with Adam and/or Keith if I don't get any answers. > > Keith very kindly put together a patch to add write block handlers for us: > http://lists.x.org/archives/xorg-devel/2012-June/031783.html > > It not only lets us handler write blocks properly, but it also lets us > remove our select and tidy our code a bit (or at least I think it does :-/). > > I've attached a patch, for discussion only (needs configure checks etc), > to see if this all makes sense to everyone else as well. > > Other eyeballs appreciated. Excellent! Patch looks good at first pass. I'll try to test it later. > > Cheers, > > Jeremy > diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c > index e57fb91..2007970 100644 > --- a/src/spiceqxl_main_loop.c > +++ b/src/spiceqxl_main_loop.c > @@ -220,19 +220,37 @@ static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void * > ring_item_init(&watch->link); > ring_add(&watches, &watch->link); > watch_count++; > + > + if (watch->event_mask & SPICE_WATCH_EVENT_WRITE) > + AddWriteSocket(watch->fd); > + if (watch->event_mask & SPICE_WATCH_EVENT_READ) > + AddGeneralSocket(watch->fd); > + > return watch; > } > > static void watch_update_mask(SpiceWatch *watch, int event_mask) > { > DPRINTF(0, "fd %d to %d", watch->fd, event_mask); > + if ((watch->event_mask & SPICE_WATCH_EVENT_WRITE) && !(event_mask & SPICE_WATCH_EVENT_WRITE)) > + RemoveWriteSocket(watch->fd); > + if ((watch->event_mask & SPICE_WATCH_EVENT_READ) && !(event_mask & SPICE_WATCH_EVENT_READ)) > + RemoveGeneralSocket(watch->fd); > watch->event_mask = event_mask; > + if (watch->event_mask & SPICE_WATCH_EVENT_WRITE) > + AddWriteSocket(watch->fd); > + if (watch->event_mask & SPICE_WATCH_EVENT_READ) > + AddGeneralSocket(watch->fd); > } > > static void watch_remove(SpiceWatch *watch) > { > DPRINTF(0, "remove %p (fd %d)", watch, watch->fd); > watch->remove = TRUE; > + if (watch->event_mask & SPICE_WATCH_EVENT_WRITE) > + RemoveWriteSocket(watch->fd); > + if (watch->event_mask & SPICE_WATCH_EVENT_READ) > + RemoveGeneralSocket(watch->fd); > watch_count--; > } > > @@ -241,70 +259,21 @@ static void channel_event(int event, SpiceChannelEventInfo *info) > NOT_IMPLEMENTED > } > > -static int set_watch_fds(fd_set *rfds, fd_set *wfds) > +static void check_watches(int retval) > { > SpiceWatch *watch; > RingItem *link; > RingItem *next; > - int max_fd = -1; > - > - RING_FOREACH_SAFE(link, next, &watches) { > - watch = (SpiceWatch*)link; > - if (watch->event_mask & SPICE_WATCH_EVENT_READ) { > - FD_SET(watch->fd, rfds); > - max_fd = watch->fd > max_fd ? watch->fd : max_fd; > - } > - if (watch->event_mask & SPICE_WATCH_EVENT_WRITE) { > - FD_SET(watch->fd, wfds); > - max_fd = watch->fd > max_fd ? watch->fd : max_fd; > - } > - } > - return max_fd; > -} > > -/* > - * called just before the X server goes into select() > - * readmask is just an fdset on linux, but something totally different on windows (etc). > - * DIX has a comment about it using a special type to hide this (so we break that here) > - */ > -static void xspice_block_handler(pointer data, OSTimePtr timeout, pointer readmask) > -{ > - /* set all our fd's */ > - set_watch_fds((fd_set*)readmask, (fd_set*)readmask); > -} > - > -/* > - * xserver only calles wakeup_handler with the read fd_set, so we > - * must either patch it or do a polling select ourselves, this is the > - * later approach. Since we are already doing a polling select, we > - * already select on all (we could avoid selecting on the read since > - * that *is* actually taken care of by the wakeup handler). > - */ > -static void select_and_check_watches(void) > -{ > - fd_set rfds, wfds; > - int max_fd = -1; > - SpiceWatch *watch; > - RingItem *link; > - RingItem *next; > - struct timeval timeout; > - int retval; > - > - FD_ZERO(&rfds); > - FD_ZERO(&wfds); > - max_fd = set_watch_fds(&rfds, &wfds); > - watch = (SpiceWatch*)watches.next; > - timeout.tv_sec = timeout.tv_usec = 0; > - retval = select(max_fd + 1, &rfds, &wfds, NULL, &timeout); > if (retval) { > RING_FOREACH_SAFE(link, next, &watches) { > watch = (SpiceWatch*)link; > if ((watch->event_mask & SPICE_WATCH_EVENT_READ) > - && FD_ISSET(watch->fd, &rfds)) { > + && CheckReadSocket(watch->fd)) { > watch->func(watch->fd, SPICE_WATCH_EVENT_READ, watch->opaque); > } > if (!watch->remove && (watch->event_mask & SPICE_WATCH_EVENT_WRITE) > - && FD_ISSET(watch->fd, &wfds)) { > + && CheckWriteSocket(watch->fd)) { > watch->func(watch->fd, SPICE_WATCH_EVENT_WRITE, watch->opaque); > } > if (watch->remove) { > @@ -315,27 +284,15 @@ static void select_and_check_watches(void) > } > } > > -static int no_write_watches(Ring *w) > +static void xspice_block_handler(pointer data, OSTimePtr timeout, pointer readmask) > { > - SpiceWatch *watch; > - RingItem *link; > - RingItem *next; > - > - RING_FOREACH_SAFE(link, next, w) { > - watch = (SpiceWatch*)link; > - if (!watch->remove && (watch->event_mask & SPICE_WATCH_EVENT_WRITE)) > - return 0; > - } > - > - return 1; > + /* We need a non-screen wakeup handler, and if you want one of those, you > + have to provide a block handler as well... */ > } > > static void xspice_wakeup_handler(pointer data, int nfds, pointer readmask) > { > - if (!nfds && no_write_watches(&watches)) { > - return; > - } > - select_and_check_watches(); > + check_watches(nfds); > } > > SpiceCoreInterface *basic_event_loop_init(void) > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel