|
From: | Marc-André Lureau |
Subject: | Re: [Qemu-devel] [PATCH 02/12] char: add qemu_chr_fe_event() |
Date: | Fri, 21 Jun 2013 10:40:44 +0200 |
Hi,
No way. You are passing an qemu-internal value (event) to an external
> +static void spice_chr_fe_event(struct CharDriverState *chr, int event)
> +{
> +#if SPICE_SERVER_VERSION >= 0x000c02
> + SpiceCharDriver *s = chr->opaque;
> +
> + spice_server_port_event(&s->sin, event);
> +#endif
> +}
library here. That is going to cause major grief in case the internal
change some day, so please don't.
I'd suggest to have something like this instead:
switch (event) {
case OPEN:
spice_server_port_open()
break;
[ ... ]
}
cheers,
Gerd
PS: Small historical lesson: spice-server 0.4.x (IIRC) was full of
these, which was a major blocker of the upstream merge of spice
support. spice-server 0.6.x got a radically different library
interface to fix that. A few places escaped review, so we still
have this in a few minor places, mouse button numbering for
example. Luckily this is a place where changes are unlikely.
But please don't let us add new ones.
[Prev in Thread] | Current Thread | [Next in Thread] |