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

Various fixes #9

Merged
merged 5 commits into from
Apr 13, 2016
Merged

Various fixes #9

merged 5 commits into from
Apr 13, 2016

Conversation

polachok
Copy link
Collaborator

These were mostly uncovered by ctest.
The biggest problem is in netmap_ring, i.e. functions operating on slots don't work without it.
ctest still reports alignment problems, but it looks like this is impossible to solve in today's rust (rust-lang/rfcs#1358).

@@ -1,6 +1,5 @@
use libc::{c_int, c_uint, c_ulong, c_char, timeval, ssize_t};
use libc::{c_int, c_uint, c_ulong, c_char, timeval, ssize_t, IF_NAMESIZE };
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove the space before the } here please?

@mrmonday
Copy link
Contributor

If you follow that thread through, it looks like there's a workaround for getting alignment right, using simd types - rust-lang/rfcs#325 (comment) - is this an option here?

@polachok
Copy link
Collaborator Author

Simd is experimental and not available on stable rust.

@mrmonday
Copy link
Contributor

I guess the real question is: do the alignment issues matter? Does it actually affect anything on the Rust side?

If yes, perhaps it would be worth just using an array, and providing accessors/mutators to get at the misaligned fields, at least until Rust has native support?

@polachok
Copy link
Collaborator Author

As far as I understand it's only a performance optimization and matters only in case we allocated it on the stack. We receive a pointer to it, so alignment is not our business.

@mrmonday
Copy link
Contributor

As long as everything works correctly on our end I'm happy. Thank you again for your work on this.

@mrmonday mrmonday merged commit 73d06ad into libpnet:master Apr 13, 2016
@polachok polachok deleted the various-fixes branch April 14, 2016 07:05
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.

2 participants