[Spice-devel] [PATCH spice-gtk 3/6] spice-channel: replace the xmit buf by a queue of SpiceOutMsg-s

Hans de Goede hdegoede at redhat.com
Sat Dec 3 01:53:15 PST 2011


Hi,

On 12/02/2011 05:44 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Dec 1, 2011 at 4:48 PM, Hans de Goede<hdegoede at redhat.com>  wrote:
>> +    if (buffered) {
>> +        spice_msg_out_ref(out);
>> +        g_queue_push_tail(&c->xmit_queue, out);
>
> Have you actually checked that the msg is not modified/reused by any
> of the callers?

Yes I checked, all the callers immediately unref it after calling
spice_msg_out_send(), so I was thinking about making it take ownership too.

But there is one exception in channel-smartcard.c, which delays the unref
until it gets a reply back from the server, which is why I decided to not go
with my make spice_msg_out_send take ownership plan. I must admit I did not
think about the possibility that channel-smartcard.c may be re-using it and
that indeed would be bad, let me check ...

Ok, no problem, channel-smartcard.c puts the  SpiceOutMsg into a struct
together with a message type, and all it cares about after the
spice_msg_out_send is the message-type, so the code can be changed to
unref the SpiceOutMsg directly after spice_msg_out_send, at which
point spice_msg_out_send can be changed to take ownership.

I'll do a revised patchset with this change.

> Instead of adding a reference, we should take ownership of the msg,
> that would be more obvious that it's not supposed to be touched once
> you call the send function.
>

Regards,

Hans


More information about the Spice-devel mailing list