Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

unix, win: add netmask to uv_interface_address #705

Closed
wants to merge 1 commit into from

Conversation

wanderview
Copy link
Contributor

Include the netmask when returning information about the OS network
interfaces. The end goal is to get this back to node's os.networkInterfaces()
function.

This commit provides implementations for windows and those unix platforms
using getifaddrs().

AIX was not implemented because it requires the use ioctls and I do not
have an AIX development/test environment. The windows code was developed
using mingw on winxp as I do not have access to visual studio.

Tested on darwin (ipv4/ipv6) and winxp (ipv4 only). Needs testing on
newer windows using ipv6 and other unix platforms.

Include the netmask when returning information about the OS network
interfaces.

This commit provides implementations for windows and those unix platforms
using getifaddrs().

AIX was not implemented because it requires the use ioctls and I do not
have an AIX development/test environment.  The windows code was developed
using mingw on winxp as I do not have access to visual studio.

Tested on darwin (ipv4/ipv6) and winxp (ipv4 only).  Needs testing on
newer windows using ipv6 and other unix platforms.
wanderview added a commit to wanderview/node that referenced this pull request Feb 10, 2013
This commit depends on joyent/libuv#705 in order to compile.  The libuv
changes are not included here as I assume dependencies are incorporated
under a separate process.

This is a partial fix for nodejs#3765.
@bnoordhuis
Copy link
Contributor

UNIX side of things LGTM.

@piscisaureus will have to sign off on the Windows changes but he probably won't be around for the next few days.

@wanderview
Copy link
Contributor Author

Thanks for the review. Note, I'd be willing to look at AIX if there was a VM or server I could use. How do you normally test AIX?

@bnoordhuis
Copy link
Contributor

We don't test AIX at all. Someone else wrote the port, we don't really support it.

@wanderview
Copy link
Contributor Author

@bnoordhuis If the windows code will take longer to review, would it make sense to just zero the values for now as you suggested in #730?

@wanderview
Copy link
Contributor Author

Ping @piscisaureus and @bnoordhuis now that we've gotten past the push for node v0.10.0. Willing to do more work on this if requested.

Thanks!

@bnoordhuis
Copy link
Contributor

Thanks Ben, landed in 14aa615.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants