[Spice-devel] [PATCH spice 2/2] server: add "port" channel support

[Hans de Goede]

Hi,

On 12/03/2012 07:37 PM, Marc-André Lureau wrote:
> Hi Hans,
>
>
> On Sun, Dec 2, 2012 at 12:26 PM, Hans de Goede <hdegoede at redhat.com <mailto:hdegoede at redhat.com>> wrote:
>
>     I think that you also need to change spicevmc_red_channel_alloc_msg_rcv_buf,
>     because now it will allocate a spice_char_device_write_buffer to hold the
>     event message, which seems the wrong thing to do, and since when handling
>     the event message you don't set  state->recv_from_client_buf = NULL, the
>     next message received on the channel will trigger this assert:
>
>          assert(!state->recv_from___client_buf);
>
>     In spicevmc_red_channel_alloc___msg_rcv_buf, I think this does not happen
>     in your testing since you only close the port once, and then send no
>     more data.
>
> It's a valid concern but it doesn't happen, because it is released by the following callback:
>
> (gdb) bt
> #0  spicevmc_red_channel_release_msg_rcv_buf (rcc=0x55555666a3f0, type=201, size=1,
>      msg=0x5555568f0330 "\002\327q\363\377\177") at spicevmc.c:338
> #1  0x00007ffff40df010 in red_peer_handle_incoming (stream=0x555556932770,
>      handler=0x55555666e500) at red_channel.c:280
> #2  0x00007ffff40df0ae in red_channel_client_receive (rcc=0x55555666a3f0)
>      at red_channel.c:294
> #3  0x00007ffff40e1e44 in red_channel_client_event (fd=28, event=1,
>      data=0x55555666a3f0) at red_channel.c:1204SPICE_MSG_PORT_INIT, item);
>

Ah right, still using a spice_char_device_write_buffer to store the event byte
feels very wrong to me, it may happen to work, but it is not what a
spice_char_device_write_buffer is intended for and may well cause weird side
effects in the future.

Regards,

Hans