<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 21, 2013 at 9:35 AM, Gerd Hoffmann <span dir="ltr"><<a href="mailto:kraxel@redhat.com" target="_blank">kraxel@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">  Hi,<br>
<div class="im"><br>
> +static void spice_chr_fe_event(struct CharDriverState *chr, int event)<br>
> +{<br>
> +#if SPICE_SERVER_VERSION >= 0x000c02<br>
> +    SpiceCharDriver *s = chr->opaque;<br>
> +<br>
> +    spice_server_port_event(&s->sin, event);<br>
> +#endif<br>
> +}<br>
<br>
</div>No way.  You are passing an qemu-internal value (event) to an external<br>
library here.  That is going to cause major grief in case the internal<br>
change some day, so please don't.<br>
<br>
I'd suggest to have something like this instead:<br>
<br>
    switch (event) {<br>
    case OPEN:<br>
        spice_server_port_open()<br>
        break;<br>
    [ ... ]<br>
    }<br></blockquote><div><br></div><div>Oh yeah, agreed.<br><br></div><div>Since the Spice server API already has spice_server_port_event() and events, I will change it to:<br><br>switch (event) {<br></div><div>case OPEN:<br>
    spice_server_port_open(SPICE_PORT_EVENT_OPENED)<br></div><div> <br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
cheers,<br>
  Gerd<br>
<br>
PS: Small historical lesson:  spice-server 0.4.x (IIRC) was full of<br>
    these, which was a major blocker of the upstream merge of spice<br>
    support.  spice-server 0.6.x got a radically different library<br>
    interface to fix that.  A few places escaped review, so we still<br>
    have this in a few minor places, mouse button numbering for<br>
    example.  Luckily this is a place where changes are unlikely.<br>
    But please don't let us add new ones.<br>
<br>
</blockquote></div><br><br clear="all"><br>-- <br>Marc-André Lureau  <br></div></div>