[Spice-devel] [spice-gtk v1 2/4] FIXUP suggestion: drop the function instead

Frediano Ziglio fziglio at redhat.com
Tue May 21 09:22:39 UTC 2019


> 
> From: Victor Toso <me at victortoso.com>
> 
> In this patch series, spice_usbutil_libusb_strerror() becomes useless
> in favor of libusb_error_name(). Instead of making the helper function
> to call the native one, drop the usage of the helper function
> entirely.
> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>

Are we sure we want libusb_error_name and not libusb_strerror ?

I think (not tested) libusb_error_name give more a "C" version
of the error, not a string to show to the user.

> ---
>  src/channel-usbredir.c   | 2 +-
>  src/usb-device-manager.c | 8 ++++----
>  src/usbutil.c            | 6 ------
>  src/usbutil.h            | 1 -
>  src/win-usb-dev.c        | 4 ++--
>  5 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 1910ff6..079a976 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -292,7 +292,7 @@ static gboolean spice_usbredir_channel_open_device(
>      if (rc != 0) {
>          g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>                      "Could not open usb device: %s [%i]",
> -                    spice_usbutil_libusb_strerror(rc), rc);
> +                    libusb_error_name(rc), rc);
>          return FALSE;
>      }
>  
> diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> index bd42142..c1df977 100644
> --- a/src/usb-device-manager.c
> +++ b/src/usb-device-manager.c
> @@ -281,7 +281,7 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable  *initable,
>      /* Initialize libusb */
>      rc = libusb_init(&priv->context);
>      if (rc < 0) {
> -        const char *desc = spice_usbutil_libusb_strerror(rc);
> +        const char *desc = libusb_error_name(rc);
>          g_warning("Error initializing USB support: %s [%i]", desc, rc);
>          g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>                      "Error initializing USB support: %s [%i]", desc, rc);
> @@ -308,7 +308,7 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable  *initable,
>          LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
>          spice_usb_device_manager_hotplug_cb, self, &priv->hp_handle);
>      if (rc < 0) {
> -        const char *desc = spice_usbutil_libusb_strerror(rc);
> +        const char *desc = libusb_error_name(rc);
>          g_warning("Error initializing USB hotplug support: %s [%i]", desc,
>          rc);
>          g_set_error(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>                    "Error initializing USB hotplug support: %s [%i]", desc,
>                    rc);
> @@ -730,7 +730,7 @@ static gboolean
> spice_usb_device_manager_get_device_descriptor(
>  
>          bus = libusb_get_bus_number(libdev);
>          addr = libusb_get_device_address(libdev);
> -        errstr = spice_usbutil_libusb_strerror(errcode);
> +        errstr = libusb_error_name(errcode);
>          g_warning("cannot get device descriptor for (%p) %d.%d -- %s(%d)",
>                    libdev, bus, addr, errstr, errcode);
>          return FALSE;
> @@ -1068,7 +1068,7 @@ static gpointer
> spice_usb_device_manager_usb_ev_thread(gpointer user_data)
>      while (g_atomic_int_get(&priv->event_thread_run)) {
>          rc = libusb_handle_events(priv->context);
>          if (rc && rc != LIBUSB_ERROR_INTERRUPTED) {
> -            const char *desc = spice_usbutil_libusb_strerror(rc);
> +            const char *desc = libusb_error_name(rc);
>              g_warning("Error handling USB events: %s [%i]", desc, rc);
>              break;
>          }
> diff --git a/src/usbutil.c b/src/usbutil.c
> index 4aa6ef7..5052ef3 100644
> --- a/src/usbutil.c
> +++ b/src/usbutil.c
> @@ -58,12 +58,6 @@ static GMutex usbids_load_mutex;
>  static int usbids_vendor_count = 0; /* < 0: failed, 0: empty, > 0: loaded */
>  static usb_vendor_info *usbids_vendor_info = NULL;
>  
> -G_GNUC_INTERNAL
> -const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
> -{
> -    return libusb_error_name(error_code);
> -}
> -
>  #ifdef __linux__
>  /* <Sigh> libusb does not allow getting the manufacturer and product strings
>     without opening the device, so grab them directly from sysfs */
> diff --git a/src/usbutil.h b/src/usbutil.h
> index de5e92a..50e3949 100644
> --- a/src/usbutil.h
> +++ b/src/usbutil.h
> @@ -28,7 +28,6 @@
>  
>  G_BEGIN_DECLS
>  
> -const char *spice_usbutil_libusb_strerror(enum libusb_error error_code);
>  void spice_usb_util_get_device_strings(int bus, int address,
>                                         int vendor_id, int product_id,
>                                         gchar **manufacturer, gchar
>                                         **product);
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index a4dfa78..ce599b9 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -113,7 +113,7 @@ g_udev_client_list_devices(GUdevClient *self, GList
> **devs,
>  
>      rc = libusb_get_device_list(priv->ctx, &lusb_list);
>      if (rc < 0) {
> -        const char *errstr = spice_usbutil_libusb_strerror(rc);
> +        const char *errstr = libusb_error_name(rc);
>          g_warning("%s: libusb_get_device_list failed - %s", name, errstr);
>          g_set_error(err, G_UDEV_CLIENT_ERROR, G_UDEV_CLIENT_LIBUSB_FAILED,
>                      "%s: Error getting device list from libusb: %s
>                      [%"G_GSSIZE_FORMAT"]",
> @@ -170,7 +170,7 @@ g_udev_client_initable_init(GInitable *initable,
> GCancellable *cancellable,
>  
>      rc = libusb_init(&priv->ctx);
>      if (rc < 0) {
> -        const char *errstr = spice_usbutil_libusb_strerror(rc);
> +        const char *errstr = libusb_error_name(rc);
>          g_warning("Error initializing USB support: %s [%i]", errstr, rc);
>          g_set_error(err, G_UDEV_CLIENT_ERROR, G_UDEV_CLIENT_LIBUSB_FAILED,
>                      "Error initializing USB support: %s [%i]", errstr, rc);

Frediano


More information about the Spice-devel mailing list