[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abst
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC PATCH v5 01/14] util: introduce gsource event abstraction |
Date: |
Mon, 29 Apr 2013 10:00:56 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Sat, Apr 27, 2013 at 10:11:40AM +0800, liu ping fan wrote:
> On Fri, Apr 26, 2013 at 5:19 PM, Stefan Hajnoczi <address@hidden> wrote:
> > On Fri, Apr 26, 2013 at 10:47:22AM +0800, Liu Ping Fan wrote:
> >> +GPollFD *events_source_add_gfd(EventsGSource *src, int fd)
> >> +{
> >> + GPollFD *retfd;
> >> +
> >> + retfd = g_slice_alloc(sizeof(GPollFD));
> >> + retfd->events = 0;
> >> + retfd->fd = fd;
> >> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
> >> + if (fd > 0) {
> >
> > 0 (stdin) is a valid fd number. Maybe just assert(fd >= 0)?
> >
> Yes, 0 should be allowed. Here, the reason to use check instead of
> assert , is that socreate() in slirp is a good place to call
> _add_gfd, but unfortunately, at that time, its socket handler so->s =
> -1; So we create the GPollFD ahead, and delay to call
> g_source_add_poll()
ok
> >> +static gboolean events_source_check(GSource *src)
> >> +{
> >> + EventsGSource *nsrc = (EventsGSource *)src;
> >> + GList *cur;
> >> + GPollFD *gfd;
> >> +
> >> + cur = nsrc->pollfds_list;
> >> + while (cur) {
> >> + gfd = cur->data;
> >> + if (gfd->fd > 0 && (gfd->revents & gfd->events)) {
> >
> > revents will always be 0 if fd is invalid, since we didn't call
> > g_source_add_poll(). Is there a reason to perform the fd > 0 check
> > again?
> >
> As explained above, we should skip the case gfd->fd=-1, which may
> occur in pollfds_list.
ok
> >> +void events_source_release(EventsGSource *src)
> >> +{
> >
> > assert that pollfds_list is empty? We don't g_slice_free() GPollFDs so
> > it must be empty here.
>
> Do you mean that events_source_add_gfd/events_source_remove_gfd are
> paired, so here, we ensure that pollfds_list==NULL ?
Yes. It is an error to hold a GPollFD across events_source_release() so
you could place an assert() here to check.
Stefan
- [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 02/14] net: introduce bind_ctx to NetClientInfo, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 03/14] net: port tap onto GSource, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 04/14] net: port vde onto GSource, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 06/14] net: port tap-win32 onto GSource, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 07/14] net: hub use lock to protect ports list, Liu Ping Fan, 2013/04/25