Instead of scanning the monitor twice (one to compute the size and another to build the message) use a single function to marshall the message. This also fixes big endian machines (which are not supported). Marshall function is exported to make easier to test (see following patch). Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- Also the reds_marshall_device_display_info function will be potentially used to separate VDIPort code. --- server/reds.c | 143 ++++++++++++++++++++++---------------------------- server/reds.h | 3 ++ 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/server/reds.c b/server/reds.c index 86f020a87..c16655adf 100644 --- a/server/reds.c +++ b/server/reds.c @@ -897,78 +897,33 @@ static RedPipeItem *vdi_port_read_one_msg_from_device(RedCharDevice *self, return NULL; } -void reds_send_device_display_info(RedsState *reds) +void reds_marshall_device_display_info(RedsState *reds, SpiceMarshaller *m) { - if (!reds->agent_dev->priv->agent_attached) { - return; - } - g_debug("Sending device display info to the agent:"); - QXLInstance *qxl; RedCharDevice *dev; - size_t message_size = sizeof(VDAgentGraphicsDeviceInfo); - - // size for the qxl device info - FOREACH_QXL_INSTANCE(reds, qxl) { - message_size += - (sizeof(VDAgentDeviceDisplayInfo) + strlen(red_qxl_get_device_address(qxl)) + 1) * - red_qxl_get_monitors_count(qxl); - } - - // size for the stream device info - GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) { - if (IS_STREAM_DEVICE(dev)) { - size_t device_address_len = - strlen(stream_device_get_device_display_info(STREAM_DEVICE(dev))->device_address); - - if (device_address_len == 0) { - // the device info wasn't set (yet), don't send it - continue; - } - - message_size += (sizeof(VDAgentDeviceDisplayInfo) + device_address_len + 1); - } - } - - RedCharDeviceWriteBuffer *char_dev_buf = vdagent_new_write_buffer(reds->agent_dev, - VD_AGENT_GRAPHICS_DEVICE_INFO, - message_size, - true); - - if (!char_dev_buf) { - reds->pending_device_display_info_message = true; - return; - } - - VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf->buf; - VDAgentGraphicsDeviceInfo *graphics_device_info = &internal_buf->u.graphics_device_info; - graphics_device_info->count = 0; - - VDAgentDeviceDisplayInfo *device_display_info = graphics_device_info->display_info; + uint32_t device_count = 0; + void *device_count_ptr = spice_marshaller_add_uint32(m, device_count); // add the qxl devices to the message FOREACH_QXL_INSTANCE(reds, qxl) { + const char *const device_address = red_qxl_get_device_address(qxl); + const size_t device_address_len = strlen(device_address) + 1; + if (device_address_len == 1) { + continue; + } for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i) { - device_display_info->channel_id = qxl->id; - device_display_info->monitor_id = i; - device_display_info->device_display_id = red_qxl_get_device_display_ids(qxl)[i]; - - strcpy((char*) device_display_info->device_address, red_qxl_get_device_address(qxl)); - - device_display_info->device_address_len = - strlen((char*) device_display_info->device_address) + 1; - - g_debug(" (qxl) channel_id: %u monitor_id: %u, device_address: %s, device_display_id: %u", - device_display_info->channel_id, - device_display_info->monitor_id, - device_display_info->device_address, - device_display_info->device_display_id); - - device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + - sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len); - - graphics_device_info->count++; + spice_marshaller_add_uint32(m, qxl->id); + spice_marshaller_add_uint32(m, i); + spice_marshaller_add_uint32(m, red_qxl_get_device_display_ids(qxl)[i]); + spice_marshaller_add_uint32(m, device_address_len); + spice_marshaller_add(m, (void*) device_address, device_address_len); + ++device_count; + + g_debug(" (qxl) channel_id: %u monitor_id: %zu, device_address: %s, " + "device_display_id: %u", + qxl->id, i, device_address, + red_qxl_get_device_display_ids(qxl)[i]); } } @@ -977,9 +932,9 @@ void reds_send_device_display_info(RedsState *reds) if (IS_STREAM_DEVICE(dev)) { StreamDevice *stream_dev = STREAM_DEVICE(dev); const StreamDeviceDisplayInfo *info = stream_device_get_device_display_info(stream_dev); - size_t device_address_len = strlen(info->device_address); + size_t device_address_len = strlen(info->device_address) + 1; - if (device_address_len == 0) { + if (device_address_len == 1) { // the device info wasn't set (yet), don't send it continue; } @@ -990,25 +945,53 @@ void reds_send_device_display_info(RedsState *reds) continue; } - device_display_info->channel_id = channel_id; - device_display_info->monitor_id = info->stream_id; - device_display_info->device_display_id = info->device_display_id; + spice_marshaller_add_uint32(m, channel_id); + spice_marshaller_add_uint32(m, info->stream_id); + spice_marshaller_add_uint32(m, info->device_display_id); + spice_marshaller_add_uint32(m, device_address_len); + spice_marshaller_add(m, (void*) info->device_address, device_address_len); + ++device_count; + + g_debug(" (stream) channel_id: %u monitor_id: %u, device_address: %s, " + "device_display_id: %u", + channel_id, info->stream_id, info->device_address, + info->device_display_id); + } + } + spice_marshaller_set_uint32(m, device_count_ptr, device_count); +} - strcpy((char*) device_display_info->device_address, info->device_address); - device_display_info->device_address_len = device_address_len + 1; +void reds_send_device_display_info(RedsState *reds) +{ + if (!reds->agent_dev->priv->agent_attached) { + return; + } + g_debug("Sending device display info to the agent:"); - g_debug(" (stream) channel_id: %u monitor_id: %u, device_address: %s, device_display_id: %u", - device_display_info->channel_id, - device_display_info->monitor_id, - device_display_info->device_address, - device_display_info->device_display_id); + SpiceMarshaller *m = spice_marshaller_new(); + reds_marshall_device_display_info(reds, m); - device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info + - sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len); + RedCharDeviceWriteBuffer *char_dev_buf = vdagent_new_write_buffer(reds->agent_dev, + VD_AGENT_GRAPHICS_DEVICE_INFO, + spice_marshaller_get_total_size(m), + true); - graphics_device_info->count++; - } + if (!char_dev_buf) { + spice_marshaller_destroy(m); + reds->pending_device_display_info_message = true; + return; + } + + VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf->buf; + + int free_info; + size_t len_info; + uint8_t *info = spice_marshaller_linearize(m, 0, &len_info, &free_info); + memcpy(&internal_buf->u.graphics_device_info, info, len_info); + if (free_info) { + free(info); } + spice_marshaller_destroy(m); reds->pending_device_display_info_message = false; diff --git a/server/reds.h b/server/reds.h index e3641fa17..8c5ee84d0 100644 --- a/server/reds.h +++ b/server/reds.h @@ -100,6 +100,9 @@ SpiceCoreInterfaceInternal* reds_get_core_interface(RedsState *reds); void reds_update_client_mouse_allowed(RedsState *reds); MainDispatcher* reds_get_main_dispatcher(RedsState *reds); +/* Marshal VDAgentGraphicsDeviceInfo structure */ +void reds_marshall_device_display_info(RedsState *reds, SpiceMarshaller *m); + /* Get the recording object stored in RedsState. * You should free with red_record_unref. */ -- 2.20.1 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel