Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: when usb debug is on in android, libusb cannot open the android device #1368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dayo7116
Copy link

@dayo7116 dayo7116 commented Dec 8, 2023

If the usb debug is on in android device, libusb fails to open the android device on windows.

This PR fixes the problem. Please Review

…oid device

Signed-off-by: dayo7116 <lidayongg@gmail.com>
@tormodvolden
Copy link
Contributor

Please provide a description of what this change does in general, not only the problem that you are seeing. Also explain what this has to do with "usb debug is on in android device".

@@ -2708,9 +2708,11 @@ static int winusbx_open(int sub_api, struct libusb_device_handle *dev_handle)
default:
return LIBUSB_ERROR_IO;
}
} else if (INVALID_HANDLE_VALUE == file_handle){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bit confusing that you use Yoda syntax here, while using normal syntax in line 2701.

@Youw
Copy link
Member

Youw commented Dec 14, 2023

I believe the former logic was:

  • try to open all interfaces;
  • if any of them fails to open - fail the entire winusbx_open;

After the update:

  • try to open all interfaces;
  • if at least one (first) interface successfully opened - ignore all subsequent interface open failures;

The new implementation is a bit confusing to me. I'd replace file_handle with a bool flag instead.

Since the new logic no longer guarantees that handle_priv->interface_handle[i].dev_handle has only valid values, something else may break in a different place. Don't know the code base but it should be considered and checked.

@mcuee mcuee added the windows label Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants