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

Fix panics and unsafe code #7

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

geigerzaehler
Copy link
Contributor

@geigerzaehler geigerzaehler commented Jul 2, 2020

This change fixes panics (#6) and unsafe code (#5). This comes at the cost of an additional copy of the data send through the pipe and having a buffer in the state. All unsafe code is removed and the need for a custom Drop implementation which makes the code overall easier.

We also provide an implementation for traits from futures which is behind a feature flag.

We also add tests.

@geigerzaehler geigerzaehler force-pushed the fix-panics-and-crashes branch from 6a0971b to 2181aa8 Compare July 2, 2020 13:53
@seanpianka
Copy link
Member

I think unsafe is used for performance reasons? The copy is unnecessary if unsafe can be shown to behave safely.

@rousan
Copy link
Member

rousan commented Feb 2, 2021

@seanpianka, would you like to be a contributor to this repo, so that you can continue to work with the routerify project easily.

@rousan
Copy link
Member

rousan commented Jul 24, 2021

@geigerzaehler, Could you please resolve the merge conflict and resend the PR?

@geigerzaehler
Copy link
Contributor Author

I’ve updated the code and the description.

@gsserge
Copy link
Member

gsserge commented Jul 28, 2021

@geigerzaehler Apologies for the trouble, but could you please resolve the conflicts so we can finally merge this?

@gsserge gsserge mentioned this pull request Jul 28, 2021
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state. All unsafe code is removed and the need for a
custom `Drop` implementation which makes the code overall easier.

We also provide an implementation for traits from `futures` which is
behind a feature flag.

We also add tests.
@geigerzaehler geigerzaehler force-pushed the fix-panics-and-crashes branch from 4a749ba to 3949517 Compare July 28, 2021 10:24
@geigerzaehler
Copy link
Contributor Author

@geigerzaehler Apologies for the trouble, but could you please resolve the conflicts so we can finally merge this?

No problem. I’ve fixed the merge conflicts.

@gsserge gsserge merged commit cd927c3 into routerify:master Jul 28, 2021
@geigerzaehler geigerzaehler deleted the fix-panics-and-crashes branch July 28, 2021 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants