[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] slirp: Properly initialize pollfds_idx of new s
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH] slirp: Properly initialize pollfds_idx of new sockets |
Date: |
Fri, 22 Feb 2013 21:17:10 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130216 Thunderbird/17.0.3 |
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 <address@hidden>
> ---
>
> 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 <address@hidden>
Thanks
Laszlo
- [Qemu-devel] [PATCH v4 01/10] main-loop: fix select_ret uninitialized variable warning, (continued)
- [Qemu-devel] [PATCH v4 01/10] main-loop: fix select_ret uninitialized variable warning, Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 04/10] slirp: slirp/slirp.c coding style cleanup, Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 03/10] main-loop: switch POSIX glib integration to GPollFD, Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 02/10] main-loop: switch to g_poll() on POSIX hosts, Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 08/10] aio: extract aio_dispatch() from aio_poll(), Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 05/10] slirp: switch to GPollFD, Stefan Hajnoczi, 2013/02/20
- [Qemu-devel] [PATCH v4 06/10] iohandler: switch to GPollFD, Stefan Hajnoczi, 2013/02/20
[Qemu-devel] [PATCH v4 09/10] aio: convert aio_poll() to g_poll(3), Stefan Hajnoczi, 2013/02/20
[Qemu-devel] [PATCH v4 10/10] aio: support G_IO_HUP and G_IO_ERR, Stefan Hajnoczi, 2013/02/20
[Qemu-devel] [PATCH v4 07/10] main-loop: drop rfds/wfds/xfds for good, Stefan Hajnoczi, 2013/02/20
Re: [Qemu-devel] [PATCH v4 00/10] main-loop: switch to g_poll(3) on POSIX hosts, Laszlo Ersek, 2013/02/20
Re: [Qemu-devel] [PATCH v4 00/10] main-loop: switch to g_poll(3) on POSIX hosts, Michael S. Tsirkin, 2013/02/20