Hi,
On 08/30/2010 01:51 PM, Alon Levy wrote:
Announce capabilities message for agent/client.
v2:
- variable length field
- includes request field, true means receiver should respond with same but
with false in request.
- client sends with true if it sees agent already connected on startup,
otherwise sends with false
This smells like a race to me, why not simply always send with request true,
like in the agent ?
- agent always sends initial with true.
- removed capability enum, message types are capabilities
- added VD_AGENT_END_MESSAGE
- added VD_AGENT_CAPS_SIZE and VD_AGENT_CAPS_BYTES helpers
- fixed VD_AGENT_HAS_CAPABILITY and VD_AGENT_SET_CAPABILITY
- both client and agent now store the capabilities (in RedClient and VDAgent
instances)
diff --git a/spice/vd_agent.h b/spice/vd_agent.h
index 1fcda88..74c955f 100644
--- a/spice/vd_agent.h
+++ b/spice/vd_agent.h
@@ -64,6 +64,8 @@ enum {
VD_AGENT_REPLY,
VD_AGENT_CLIPBOARD,
VD_AGENT_DISPLAY_CONFIG,
+ VD_AGENT_ANNOUNCE_CAPABILITIES,
+ VD_AGENT_END_MESSAGE,
};
typedef struct SPICE_ATTR_PACKED VDAgentMonConfig {
@@ -129,6 +131,21 @@ enum {
VD_AGENT_CLIPBOARD_UTF8_TEXT = 1,
};
+typedef struct SPICE_ATTR_PACKED VDAgentAnnounceCapabilities {
+ uint8_t request;
Please make this an uint32_t, making this an uint8_t combined with
SPICE_ATTR_PACKED will make caps_size and caps not be 32 bit aligned,
causing performancy penalties on Intel, and segfaults / bus errors
on non Intel architectures (like the arm used in android). All cpu's
but intel do *not* allow non word aligned addresses for word accesses.
+ uint32_t caps_size;
Why the size field here, VDAgentMessage already has a size field
which can be used. On second thought, lets keep this see below.
+ uint32_t caps[0];
+} VDAgentAnnounceCapabilities;
+
+#define VD_AGENT_CAPS_SIZE ((VD_AGENT_END_MESSAGE + 31) / 32)
+
+#define VD_AGENT_CAPS_BYTES (((VD_AGENT_END_MESSAGE + 31) / 8)& (0xffffffff -
3))
+
Why not just (VD_AGENT_CAPS_SIZE * sizeof(uint32_t)) ? At least write
the mask as ~3 for readability.
+#define VD_AGENT_HAS_CAPABILITY(caps, caps_size, index) \
+ ((index)< (caps_size * 32)&& ((caps)[(index) / 32]& (1<< ((index)&
0x1f))))
+
+#define VD_AGENT_SET_CAPABILITY(caps, index) \
+ { (caps)[(index) / 32] |= (1<< ((index)& 0x1f)); }
Hmm, okay so I'm starting to see sense in having a caps_size field. in
VDAgentAnnounceCapabilities. Can we make these macro take an entire
VDAgentAnnounceCapabilities struct as argument, that would make
the use of VD_AGENT_HAS_CAPABILITY a bit easier to read.
Regards,
Hans
_______________________________________________
Spice-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/spice-devel
