[Spice-devel] [PATCH spice-gtk 2/2] Add a USB device selection widget.

[Hans de Goede]

Hi,

On 01/26/2012 04:40 PM, Christophe Fergeau wrote:
> I had a quick glance through this, this looks good apart from a few nits.
> I'll let elmarco comment on the widget part though since he is more
> experienced than I am in this.
>

Thanks for the review!

> Christophe
>
> On Thu, Jan 26, 2012 at 04:24:56PM +0100, Hans de Goede wrote:

<snip>

>> +#ifdef USE_USBREDIR
>> +static void menu_cb_select_usb_devices(GtkAction *action, void *data)
>> +{
>> +    GtkWidget *dialog, *area, *usb_device_widget;
>> +    struct spice_window *win = data;
>> +
>> +    /* Create the widgets */
>> +    dialog = gtk_dialog_new_with_buttons(
>> +                    _("Select USB Devices for redirection"),
>
> The capitalization of 'Devices' is a bit odd here
>

Agreed, fixed in my local tree.

<snip>

>> +static void spice_usb_device_widget_set_property(GObject       *gobject,
>> +                                                 guint          prop_id,
>> +                                                 const GValue  *value,
>> +                                                 GParamSpec    *pspec)
>> +{
>> +    SpiceUsbDeviceWidget *self = SPICE_USB_DEVICE_WIDGET(gobject);
>> +    SpiceUsbDeviceWidgetPrivate *priv = self->priv;
>> +
>> +    switch (prop_id) {
>> +    case PROP_SESSION:
>> +        priv->session = g_object_ref(g_value_get_object(value));
>> +        break;
>> +    case PROP_DEVICE_FORMAT_STRING:
>> +        priv->device_format_string = g_strdup(g_value_get_string(value));
>> +        break;
>
> There are g_value_dup_object and g_value_dup_string for that purpose
>

Ah didn't know about those, fixed in my local tree.

<snip>

>> +/* ------------------------------------------------------------------ */
>> +/* public api                                                         */
>> +
>> +/**
>> + * spice_usb_device_widget_new:
>> + * @session: #SpiceSession for which to widget will control USB redirection
>> + * @device_format_string: String passed to spice_usb_device_get_description()
>> + *
>> + * Returns: a new #SpiceUsbDeviceWidget instance
>> + */
>> +GtkWidget *spice_usb_device_widget_new(SpiceSession    *session,
>> +                                       gchar           *device_format_string)
>
>
> device_format_string can be const.
>

Agreed, fixed in my local tree.

<snip>

Regards,

Hans