Re: [PATCH 5/9] Add empty CD entry to usb-device-widget, create shared CD when toggled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Fri, Dec 6, 2019 at 12:18 PM Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
>
> +
> +    if (gtk_toggle_button_get_active(toggle))
> +        gtk_toggle_button_set_active(toggle, FALSE);

style: always use brackets, here and below

OK 
> +    else
> +        return;

I would handle the return in the if to reduce indentation
OK 
 
> +
> +    dialog = gtk_file_chooser_dialog_new(_("Select ISO file or device"),
> +
> GTK_WINDOW(gtk_widget_get_toplevel(GTK_WIDGET(self))),
> +                                         GTK_FILE_CHOOSER_ACTION_OPEN,
> +                                         _("_Cancel"), GTK_RESPONSE_CANCEL,
> +                                         _("_Open"), GTK_RESPONSE_ACCEPT,
> +                                         NULL);
> +
> +    dialog_rc = gtk_dialog_run(GTK_DIALOG(dialog));
> +    if (dialog_rc == GTK_RESPONSE_ACCEPT) {
> +        gchar *filename =
> gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
> +        GError *err = NULL;
> +        gboolean rc;
> +
> +        rc = spice_usb_device_manager_create_shared_cd_device(priv->manager,
> filename, &err);
> +        if (!rc && err) {
> +            gchar *err_msg = g_strdup_printf("shared CD %s, %s",
> +                                             g_path_get_basename(filename),
> err->message);

I would add _( to string for i18n
OK 

> +
> +            SPICE_DEBUG("Failed to create %s", err_msg);
> +            spice_usb_device_widget_add_err_msg(self, err_msg);
> +            spice_usb_device_widget_update_status(user_data);
> +        }
> +        g_clear_error(&err);
 Similar to the previous patch

> +    }
> +    empty_cd = gtk_check_button_new_with_label("SPICE CD (empty)");

i18n also here
OK

>  static void spice_usb_device_widget_constructed(GObject *gobject)
>  {
> . . . 
> +    priv->empty_cd = NULL;

This is not necessary, we don't support environment where NULL != 0 and all
structure is initialized to 0 as default.
And standard C decided now that NULL == 0 so no issue for the future.
OK 
 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]