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

Shared OS Behaviour #19

Merged
merged 5 commits into from
Nov 30, 2019
Merged

Shared OS Behaviour #19

merged 5 commits into from
Nov 30, 2019

Conversation

rtulip
Copy link
Contributor

@rtulip rtulip commented Nov 27, 2019

Resolves: #17

Moved the following into os/shared.rs:

  • KeyboardEvents
  • get_datalink_channel
  • get_interface
  • lookup_addr
  • sigwinch
  • create_write_to_stdout
  • get_input

Of note: get_interface had a slightly different implementation between os/linux.rs and os/macos.rs and they have been standardized in os/shared.rs. The only difference between the implementation was that os/linux.rs set the config.read_timeout.

Additionally, I noticed that there's a RawConnection struct in os/macos.rs, that wasn't being used in the file.

Also, I only have a Linux machine, and so I was unable to test the MacOS stuff, so please ensure to do that.

@imsnif
Copy link
Owner

imsnif commented Nov 29, 2019

Just to update: I've had a very busy week and plan on looking at this over the weekend. Thanks for this! Will review very soon. :)

@imsnif
Copy link
Owner

imsnif commented Nov 30, 2019

Hey @rtulip - really great work, thanks for this!

Good catch with the unused struct in macos - if you'd like, you can do another PR that removes it (otherwise I'll get to it myself at some point :) ).

I'm not sure why the timeout wasn't in the macos version. I think I added it to get a proper error message when there aren't sufficient privileges to read the network card. It should definitely be there, in any case. Just goes to show why it's good that we're merging these now!

For future reference - the macos version works on a linux machine. So if you want to test it, you can just swap macos/linux and build+run. For now though, I tested it and it looks great.

I would be happy to receive further contributions from you if you're interested. There are a bunch of open issues that I'd be very happy see handled before releasing 1.0.0. Feel free to reach out to me if you have any thoughts, ideas or are just wondering what best to work on: [email protected]

@imsnif imsnif merged commit 81bc53a into imsnif:master Nov 30, 2019
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.

Shared os behaviour
2 participants