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.

Sign up for GitHub

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

Jump to bottom

Add DeviceHandle::into_raw() #97

Merged
merged 1 commit into from Aug 11, 2021
Merged

Add DeviceHandle::into_raw() #97

merged 1 commit into from Aug 11, 2021

Conversation

elmarco
Copy link
Contributor

@elmarco elmarco commented Aug 1, 2021

As exotic as it may be, libusbredir requires to give the device handle
ownership to the library. This function helps to achieve this, without
having to leak the a DeviceHandle with mem::forget.

@a1ien
Copy link
Owner

a1ien commented Aug 2, 2021

Looks good. But I need a little time to think it over.

@elmarco
Copy link
Contributor Author

elmarco commented Aug 2, 2021

We may want to make the function explicitely unsafe?

@elmarco elmarco mentioned this pull request Aug 3, 2021
Merged
@elmarco
Copy link
Contributor Author

elmarco commented Aug 10, 2021

Following Rust idioms,I don't think the function should be unsafe.

@a1ien what do you think? thanks

a1ien
a1ien reviewed Aug 11, 2021
View reviewed changes
src/device_handle.rs Outdated Show resolved Hide resolved
@a1ien
Copy link
Owner

a1ien commented Aug 11, 2021

I don't like just one thing it's unwrap calls as_raw and into_raw.
I think match and unreachable!() wold better

@elmarco
Copy link
Contributor Author

elmarco commented Aug 11, 2021

I don't like just one thing it's unwrap calls as_raw and into_raw.
I think match and unreachable!() wold better

ok, but why?

@a1ien
Copy link
Owner

a1ien commented Aug 11, 2021 •
edited

ok, but why?

unwrap looks like not finished code. Like "example". But match and unreachable it's mean you absolutely do not expect that this path will be executed

a1ien
a1ien reviewed Aug 11, 2021
View reviewed changes
src/device_handle.rs Outdated Show resolved Hide resolved
@elmarco
Add DeviceHandle::into_raw()
52a9de9 
As exotic as it may be, libusbredir requires to give the device handle
ownership to the library. This function helps to achieve this, without
having to leak the a DeviceHandle with mem::forget.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
@a1ien
Copy link
Owner

a1ien commented Aug 11, 2021

LGTM. Thanks!

@a1ien a1ien merged commit fb0940c into a1ien:master Aug 11, 2021
@a1ien a1ien mentioned this pull request Sep 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a1ien a1ien a1ien left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@elmarco @a1ien