[Spice-devel] [PATCH 4/9] Implement spice_usb_device_widget_add_err_msg() in usb-device-widget

[Frediano Ziglio]

In the title I would say "Factor out" instead of "Implement"

> From: Alexander Nezhinsky <anezhins at redhat.com>
> 
> Common function for adding an error status message.
> 
> If an old message exists, checks if the new message is not already contained
> in the old one. The new message is ignored if it is, added if it's not.
> 
> New message string should be dynamically allocated, it's always g_free-ed.
> 
> Signed-off-by: Alexander Nezhinsky <anezhins at redhat.com>
> ---
>  src/usb-device-widget.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/src/usb-device-widget.c b/src/usb-device-widget.c
> index 55a9687..6e7a267 100644
> --- a/src/usb-device-widget.c
> +++ b/src/usb-device-widget.c
> @@ -131,6 +131,25 @@ static void spice_usb_device_widget_set_property(GObject
> *gobject,
>      }
>  }
>  
> +static void spice_usb_device_widget_add_err_msg(SpiceUsbDeviceWidget *self,
> +                                                gchar *new_err_msg)

Why not spice_usb_device_widget_append_err_msg ?
"add" seems to suggest a list of errors.

> +{
> +    SpiceUsbDeviceWidgetPrivate *priv = self->priv;
> +
> +    if (priv->err_msg) {
> +        /* Append the new error message to err_msg,
> +           but only if it's *not* already there! */
> +        if (!strstr(priv->err_msg, new_err_msg)) {
> +            gchar *old_err_msg = priv->err_msg;
> +            priv->err_msg = g_strdup_printf("%s\n%s", old_err_msg,
> new_err_msg);
> +            g_free(old_err_msg);
> +            g_free(new_err_msg);
> +        }
> +    } else {
> +        priv->err_msg = new_err_msg;
> +    }
> +}
> +
>  static void spice_usb_device_widget_hide_info_bar(SpiceUsbDeviceWidget
>  *self)
>  {
>      SpiceUsbDeviceWidgetPrivate *priv = self->priv;
> @@ -359,19 +378,10 @@ static void check_can_redirect(GtkWidget *widget,
> gpointer user_data)
>      } else {
>          can_redirect =
>          spice_usb_device_manager_can_redirect_device(priv->manager,
>                                                                      device,
>                                                                      &err);
> -        /* If we cannot redirect this device, append the error message to
> -           err_msg, but only if it is *not* already there! */
> +
>          if (!can_redirect) {
> -            if (priv->err_msg) {
> -                if (!strstr(priv->err_msg, err->message)) {
> -                    gchar *old_err_msg = priv->err_msg;
> -                    priv->err_msg = g_strdup_printf("%s\n%s", priv->err_msg,
> -                                                    err->message);
> -                    g_free(old_err_msg);
> -                }
> -            } else {
> -                priv->err_msg = g_strdup(err->message);
> -            }
> +            gchar *err_msg = g_strdup_printf("Can't redirect: %s",
> err->message);
> +            spice_usb_device_widget_add_err_msg(self, err_msg);
>          }
>          g_clear_error(&err);
>      }

Frediano