Hi,
On Tue, Jul 30, 2013 at 5:00 AM, Fedor Lyakhov <[email protected]>wrote: > Hi, Dunrong > > Thanks for the review, my comments inline. > > On Mon, Jul 29, 2013 at 6:58 AM, Dunrong Huang <[email protected]> > wrote: > > Hi, > > > > > > On Mon, Jul 29, 2013 at 4:43 AM, Fedor Lyakhov <[email protected]> > > wrote: > >> > >> --- > >> src/vdagentd-proto-strings.h | 1 + > >> src/vdagentd-proto.h | 3 ++- > >> src/vdagentd.c | 38 > ++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 41 insertions(+), 1 deletion(-) > >> > >> diff --git a/src/vdagentd-proto-strings.h b/src/vdagentd-proto-strings.h > >> index e76cb3b..7ea7195 100644 > >> --- a/src/vdagentd-proto-strings.h > >> +++ b/src/vdagentd-proto-strings.h > >> @@ -34,6 +34,7 @@ static const char * const vdagentd_messages[] = { > >> "file xfer status", > >> "file xfer data", > >> "client disconnected", > >> + "display config" > > > > Adding a comma at the end will be better, so next time someone don't > need to > > change this line when they add something. > > OK, will add it. But won't compiler give a warning on this extra comma > actually? > It's common way for array initialization, no harm to compiler. > > >> > >> }; > >> > >> #endif > >> diff --git a/src/vdagentd-proto.h b/src/vdagentd-proto.h > >> index 25e6a36..6a09e49 100644 > >> --- a/src/vdagentd-proto.h > >> +++ b/src/vdagentd-proto.h > >> @@ -39,7 +39,8 @@ enum { > >> VDAGENTD_FILE_XFER_START, > >> VDAGENTD_FILE_XFER_STATUS, > >> VDAGENTD_FILE_XFER_DATA, > >> - VDAGENTD_CLIENT_DISCONNECTED, /* daemon -> client */ > >> + VDAGENTD_CLIENT_DISCONNECTED, /* daemon -> client */ > >> + VDAGENTD_DISPLAY_CONFIG, /* daemon -> client, VDAgentDisplayConfig > */ > >> VDAGENTD_NO_MESSAGES /* Must always be last */ > >> }; > >> > >> diff --git a/src/vdagentd.c b/src/vdagentd.c > >> index f4cea44..c9df401 100644 > >> --- a/src/vdagentd.c > >> +++ b/src/vdagentd.c > >> @@ -91,6 +91,7 @@ static void send_capabilities(struct > >> vdagent_virtio_port *vport, > >> caps->request = request; > >> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MOUSE_STATE); > >> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MONITORS_CONFIG); > >> + VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_DISPLAY_CONFIG); > >> VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_REPLY); > >> VD_AGENT_SET_CAPABILITY(caps->caps, > >> VD_AGENT_CAP_CLIPBOARD_BY_DEMAND); > >> VD_AGENT_SET_CAPABILITY(caps->caps, > >> VD_AGENT_CAP_CLIPBOARD_SELECTION); > >> @@ -151,6 +152,38 @@ static void do_client_monitors(struct > >> vdagent_virtio_port *vport, int port_nr, > >> (uint8_t *)&reply, sizeof(reply)); > >> } > >> > >> +static void do_client_display(struct vdagent_virtio_port *vport, int > >> port_nr, > >> + VDAgentMessage *message_header, VDAgentDisplayConfig *disp) > >> +{ > >> + VDAgentReply reply; > >> + VDAgentDisplayConfig *display_config; > >> + uint32_t size; > >> + > >> + size = sizeof(VDAgentDisplayConfig); > >> + if (message_header->size != size) { > >> + syslog(LOG_ERR, "invalid message size for > VDAgentDisplayConfig"); > >> + return; > >> + } > >> + > >> + display_config = malloc(size); > >> > >> + if (!display_config) { > >> + syslog(LOG_ERR, "oom allocating display config"); > >> + size = 0; > >> + } > >> + memcpy(display_config, disp, size); > >> + > >> + /* Send display config to currently active agent */ > >> + if (active_session_conn) > >> + udscs_write(active_session_conn, VDAGENTD_DISPLAY_CONFIG, 0, 0, > >> + (uint8_t *)display_config, size); > > > > display_config will be leaked, why not just use the "disp"? > > Agree. Thanks for finding this. Bad copy of code from do_client_monitors(). > > >> > >> + > >> + /* Acknowledge reception of display config to spice server / client > >> */ > >> + reply.type = VD_AGENT_DISPLAY_CONFIG; > >> + reply.error = VD_AGENT_SUCCESS; > >> + vdagent_virtio_port_write(vport, port_nr, VD_AGENT_REPLY, 0, > >> + (uint8_t *)&reply, sizeof(reply)); > >> +} > >> + > >> static void do_client_capabilities(struct vdagent_virtio_port *vport, > >> VDAgentMessage *message_header, > >> VDAgentAnnounceCapabilities *caps) > >> @@ -330,6 +363,11 @@ int virtio_port_read_complete( > >> do_client_monitors(vport, port_nr, message_header, > >> (VDAgentMonitorsConfig *)data); > >> break; > >> + case VD_AGENT_DISPLAY_CONFIG: > >> + if (message_header->size < sizeof(VDAgentDisplayConfig)) > >> + goto size_error; > >> + do_client_display(vport, port_nr, message_header, > >> + (VDAgentDisplayConfig *)data); > >> case VD_AGENT_ANNOUNCE_CAPABILITIES: > >> if (message_header->size < sizeof(VDAgentAnnounceCapabilities)) > >> goto size_error; > >> -- > >> 1.8.1.4 > >> _______________________________________________ > >> Spice-devel mailing list > >> [email protected] > >> http://lists.freedesktop.org/mailman/listinfo/spice-devel > -- Best Regards, Dunrong Huang Homepage: http://mathslinux.org
_______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
