Skip to content
This repository has been archived by the owner on Sep 14, 2019. It is now read-only.

Bug 104714: implement HasIPConnected on Darwin #42

Merged
merged 3 commits into from
Mar 17, 2015

Conversation

djmitche
Copy link
Contributor

I tested this by hand on OS X 10.8.

I did run into some trouble, indicated by the comments inline, where netstat produces a truncated ipv6 address. I don't have any idea how to fix that, or if it's even possible.

@@ -62,8 +64,104 @@ func HasSeenMac(val string) (found bool, elements []element, err error) {
return
}

func parseEndpointString(str string) (ip net.IP, port int) {
re, err := regexp.Compile("^(.*?)(%[a-z0-9]+)?\\.(\\*|[0-9]+)$")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'd be nice to not re-compile this regexp every time -- tips?

Copy link
Contributor

Choose a reason for hiding this comment

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

You could define it as a global variable, since the regexp is static:

package main

import (
    "fmt"
    "regexp"
)

var re = regexp.MustCompile("^$")

func main() {
    if re.MatchString("test") {
        fmt.Println("test doesn't match")
    }
    if re.MatchString("") {
        fmt.Println("empty string matches")
    }
}

@djmitche
Copy link
Contributor Author

@jvehent pointed out http://stackoverflow.com/questions/5390164/getting-routing-table-on-macosx-programmatically -- not sure that's worth the effort..

func parseEndpointString(str string) (ip net.IP, port int) {
re, err := regexp.Compile("^(.*?)(%[a-z0-9]+)?\\.(\\*|[0-9]+)$")
if err != nil {
panic(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would recommend not panicking directly, but instead return an error value here, or panic and recover.

@jvehent
Copy link
Contributor

jvehent commented Mar 15, 2015

Looks good. Just a couple minors.

@djmitche
Copy link
Contributor Author

OK, those should be fixed up - let me know what you think

@jvehent
Copy link
Contributor

jvehent commented Mar 17, 2015

Looks good. r+

jvehent added a commit that referenced this pull request Mar 17, 2015
Bug 104714: implement HasIPConnected on Darwin
@jvehent jvehent merged commit 744ed37 into mozilla:master Mar 17, 2015
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