On 02/22/13 20:51, Jan Kiszka wrote: > Otherwise we may start processing sockets in slirp_pollfds_poll that > were created past slirp_pollfds_fill. > > Signed-off-by: Jan Kiszka <[email protected]> > --- > > Not sure if this pattern also applies to other users besides slirp. > Worth checking I suppose. > > slirp/socket.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/slirp/socket.c b/slirp/socket.c > index a7ab933..bb639ae 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -51,6 +51,7 @@ socreate(Slirp *slirp) > so->so_state = SS_NOFDREF; > so->s = -1; > so->slirp = slirp; > + so->pollfds_idx = -1; > } > return(so); > } >
Aaaargh. slirp_pollfds_fill() actually has three places where it does this: - when appending a TCP socket, - when appending a UDP socket, - when appending an ICMP socket, to the pollfds array. The assumption was (I think!) that once we've iterated over all of: - slirp->tcb - slirp->udb - slirp->icmp of each "slirp_instance", then we must have covered all slirp sockets in existence, in slirp_pollfds_fill(). I *guess* that, after we g_poll()ed, and are in slirp_pollfds_poll(), we can create (even accept maybe?) new sockets that are put on slirp->tcb / slirp->udb / slirp->icmp. That is, near the end of each of these control block lists, we can encounter sockets that weren't there when we *entered* slirp_pollfds_poll(), let alone when we called slirp_pollfds_fill() most recently! (It would be interesting to see an actual call chain when this happens.) aio-posix and iohandler seem to get this right though, I believe: - in aio_set_fd_handler(), the index is set to -1, - qemu_iohandler_fill() does the same. (We even might have called it "general hygiene" or some such?...) Of course Stefan should check :) The patch looks good to me. It surely can't hurt! Reviewed-by: Laszlo Ersek <[email protected]> Thanks Laszlo
