Hi On Sat, Feb 19, 2011 at 11:07 AM, Hans de Goede <[email protected]> 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 _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
