[Spice-devel] UsbDk-related patches

[Dmitry Fleytman]

Sent from my iPhone

> On 27 Jan 2016, at 15:58, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
>> On Wed, Jan 27, 2016 at 02:09:25PM +0200, Dmitry Fleytman wrote:
>> 
>>> On 27 Jan 2016, at 14:00 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
>>> 
>>> On Wed, Jan 27, 2016 at 01:51:12PM +0200, Dmitry Fleytman wrote:
>>>> Hello, Spice List
>>>> 
>>>> I just got a confirmation that UsbDk backend integration for libusb was pushed upstream:
>>>> 
>>>> 24c5289 build: Integrate usbdk backend
>>>> 8823c17 usbdk: Introduce usbdk backend
>>>> 5d83aba windows: Rename windows_usb.h/c windows_winusb.h/c
>>>> 92a3278 windows: Move common definitions to a separate file
>>>> 
>>>> (see at https://github.com/libusb/libusb/commits/master)
>>> 
>>> Didn't
>>> https://github.com/libusb/libusb/commit/cdfe236931ef3fd06adac0fdf6df49baa39ff35e
>>> reintroduce the bug where if there are usbdk issues,
>>> libusb_init() will fail the first time it's called, and then report
>>> success without doing any initialisation?
>> 
>> (looping Chris)
>> 
>> Hi Chris, 
>> 
>> There is an opinion that commit
>> (https://github.com/libusb/libusb/commit/cdfe236931ef3fd06adac0fdf6df49baa39ff35e)
>> is problematic (see above). If so, there is the symmetrical problem
>> with WinUsb backend as well. What do you think?
> 
> The WinUsb backend is doing things slightly differently, and looks more
> correct, something like:
> (not sure how bad it is to call window_common_exit() when windows_common_init()
> hasn't been called, this can happen with this patch if load_usbdk_helper_dll fails)

Here, it is ok to call cleanup when init fails. The idea is to avoid duplication of cleanup code.

> 
> diff --git a/libusb/os/windows_usbdk.c b/libusb/os/windows_usbdk.c
> index 9abb83e..2b97f69 100644
> --- a/libusb/os/windows_usbdk.c
> +++ b/libusb/os/windows_usbdk.c
> @@ -195,25 +195,30 @@ error_unload:
> 
> static int usbdk_init(struct libusb_context *ctx)
> {
> -       int r;
> +       int r = LIBUSB_ERROR_OTHER;
> 
>        if (++concurrent_usage == 0) {
>                r = load_usbdk_helper_dll(ctx);
>                if (r)
> -                       return r;
> +                       goto init_exit;
> 
>                init_polling();
> 
>                r = windows_common_init(ctx);
>                if (r)
> -                       goto error_roll_back;
> +                       goto init_exit;
>        }
> 
> -       return LIBUSB_SUCCESS;
> +       r = LIBUSB_SUCCESS;
> +
> +init_exit:
> +       if (!concurrent_usage && r != LIBUSB_SUCCESS) { // First init failed?
> +               windows_common_exit();
> +               unload_usbdk_helper_dll();
> +
> +       if (r != LIBUSB_SUCCESS)
> +               --concurrent_usage;
> 
> -error_roll_back:
> -       windows_common_exit();
> -       unload_usbdk_helper_dll();
>        return r;
> }
> 
> Christophe