On 11/10/2018 17:24, Philippe Mathieu-Daudé wrote:
>> On the other hand, fd_chr_read_poll is not an IOCanReadHandler, and this
>> patch therefore probably doesn't compile?
> It does compile.
>
> fd_chr_update_read_handler() uses fd_chr_read_poll with io_add_watch_poll():
>
> GSource *io_add_watch_poll(Chardev *chr,
> QIOChannel *ioc,
> IOCanReadHandler *fd_can_read,
> QIOChannelFunc fd_read,
> gpointer user_data,
> GMainContext *context);
Oh, that's somewhat weird. It could just as well return a bool.
However, this made me notice that you need to change e.g. s->max_size's
declaration (in include/chardev/char-fd.h) from int to size_t, and
likewise for: 1) all users of s->max_size, such as len in fd_chr_read;
2) all the similar variables in other char backends.
So it's probably best to structure the series as follows:
1) change fd_can_read from IOCanReadHandler to a GSourceFunc (which
returns a boolean value), changing all "return s->foo" to "return s->foo
> 0;". Then you can remove the > 0 from
bool now_active = iwp->fd_can_read(iwp->opaque) > 0;
(Having the > 0 repeated in all backends is now a bit ugly, but there
are future cleanup opportunities here to move the qemu_chr_be_can_write
call to qemu_chr_be_can_write; this way most chardev backends can skip
defining a read_poll function. But I digress).
2) assert in qemu_chr_be_can_write that the returned value is >= 0
3) for each backend, change the assigned variable from int to size_t
4) now the rest of your patch, touching all front-ends. The assertion
from (2) now does not make sense anymore, since ->can_read returns an
unsigned value, but perhaps you can keep a "fail-safe" assertion that
(ssize_t)returned_value >= 0 to catch undesired overflows.
Paolo
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
