On Fri, Feb 22, 2013 at 09:17:10PM +0100, Laszlo Ersek wrote: > 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 <jan.kis...@siemens.com> > > --- > > > > 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?...)
Right, we take care to initialize pollfds_idx = -1 in iohandler.c and aio-posix.c. Jan: Thanks for finding and fixing this, it was an oversight on my part.
