[Spice-devel] [PATCH] protocol: RFC: add clipboard selection capability

[Marc-André Lureau]

Hi

On Sat, Feb 19, 2011 at 11:07 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>> VDAgentClipboard* messages MUST be prepended with a uint8_t indicating
>> which clipboard selection to operate.
>
> I'm not sure I like this part, I would rather see new structs to use
> when this capability is present:
>
> For example a new VDAgentSelectionGrab to use instead of
> VDAgentClipboardGrab
> when both sides have the capability, which looks like this:
>
> typedef struct SPICE_ATTR_PACKED VDAgentSelectionGrab {
>    uint32_t selection;
>    uint32_t types[0];
> } VDAgentClipboardGrab;
>
> Note I made the selection a uint32_t on purpose, this way the following
> uint32_t's in the struct will not get unaligned by the packing.

That's kind of annoying, because we duplicate structure and code patch
for just an int... This is certainly a topic I would like to discuss
more :)

Tbh, I am not fond of the "struct" based approach. It's not the first
time I want to modify slightly the protocol, and it's not easy to do.

My experience with networking code was the best when working on
PulseAudio, the pa_tagstruct can contain various fields depending on
version and capabilities (take a look at
http://git.0pointer.de/?p=pulseaudio.git;a=blob;f=src/pulsecore/protocol-native.c)

This is very convenient, and the various helper functions make it easy
to put variable length strings or property list etc..

However, I realize that having the protocol defined in struct format
make it easier for other to implement, and can be used to generate
code.

> I took a look at your linux vdagent implementation for this, and it looks
> good. Except that you seem to do the prepending with the selection
> unconditionally, iow even when the other side does not have the
> VD_AGENT_CAP_CLIPBOARD_SELECTION cap.
>

Nope, the vdagentd scrap the selection byte if the client doesn't
support it. I tested with spicec (since I didn't patch the spicec
client to support it)

cheers

-- 
Marc-André Lureau