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

Linux-part of #47: Implement per-device threads, don't use the KeyHandleMatcher #56

Merged
merged 1 commit into from Nov 20, 2017

Conversation

ttaubert
Copy link
Contributor

This rewrites the Linux backend to look like the macOS one. See #52.

@ttaubert ttaubert self-assigned this Nov 20, 2017
Copy link
Contributor

@jcjones jcjones left a comment

Choose a reason for hiding this comment

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

This is a lot cleaner, well done.

src/linux/mod.rs Outdated
return;
}

// Iterate the exlude list and see if there are any matches.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/exlude/exclude/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix, thanks!

thread: Option<RunLoop>,
}

impl Transaction {
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels like this could have a common interface with macos/transaction.rs, but perhaps that should be a follow-on after whatever happens to Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I've been thinking about this too. There are tiny differences, e.g. the parameters for Device::new() and whether we call dev.is_u2f() or not but we can work around that. I agree that we should do this after converting Windows.

@@ -108,7 +108,7 @@ impl PlatformManager {
u2f_is_keyhandle_valid(dev, &challenge, &application, key_handle)
.unwrap_or(false) /* no match on failure */
})
.collect::<Vec<&Vec<u8>>>();
.collect::<Vec<_>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Odd that this wasn't caught by the linter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a tiny cleanup. I hate having to be so specific :)

@ttaubert ttaubert merged commit 7d9f31a into mozilla:master Nov 20, 2017
@ttaubert ttaubert deleted the no-khmatcher-linux branch November 20, 2017 17:04
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 20, 2017
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Nov 21, 2017
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request Nov 22, 2017
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a

UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a

UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…'t use KeyHandleMatcher r=jcj

This patch rewrites the Linux backend to have a structure similar to the macOS
one [1]. We'll have one thread per device, which means that a device that is
stuck or unresponsive won't break other devices.

[1] mozilla/authenticator-rs#52

Review: mozilla/authenticator-rs#56

mozilla/authenticator-rs@7d9f31a

UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants