Possible fix for issue #4 - segfault in ___removeIp helper function in EthernetTap on OSX -- I think the problem may have been that I was using set::erase(key) while also using an iterator, so now it uses erase(iterator). See if it happens again, cause I could not duplicate the issue. Possible minor difference in STL version.

3 files changed, 34 insertions, 27 deletions

diff --git a/node/EthernetTap.cpp b/node/EthernetTap.cpp
index 01bd72cc..596cc5c2 100644
--- a/node/EthernetTap.cpp
+++ b/node/EthernetTap.cpp
@@ -478,7 +478,8 @@ EthernetTap::~EthernetTap()
 	delete [] _putBuf;
 }
 
-static bool ___removeIp(const char *_dev,std::set<InetAddress> &_ips,const InetAddress &ip)
+// Helper function to actually remove IP from network device, execs ifconfig
+static bool ___removeIp(const char *_dev,const InetAddress &ip)
 {
 	int cpid;
 	if ((cpid = (int)fork()) == 0) {
@@ -487,11 +488,9 @@ static bool ___removeIp(const char *_dev,std::set<InetAddress> &_ips,const InetA
 	} else {
 		int exitcode = -1;
 		waitpid(cpid,&exitcode,0);
-		if (exitcode == 0) {
-			_ips.erase(ip);
-			return true;
-		} else return false;
+		return (exitcode == 0);
 	}
+	return false; // never reached, make compiler shut up about return value
 }
 
 bool EthernetTap::addIP(const InetAddress &ip)
@@ -501,13 +500,17 @@ bool EthernetTap::addIP(const InetAddress &ip)
 	if (!ip)
 		return false;
 	if (_ips.count(ip) > 0)
-		return true;
+		return true; // IP/netmask already assigned
 
 	// Remove and reconfigure if address is the same but netmask is different
 	for(std::set<InetAddress>::iterator i(_ips.begin());i!=_ips.end();++i) {
-		if (i->ipsEqual(ip)) {
-			___removeIp(_dev,_ips,*i);
-			break;
+		if ((i->ipsEqual(ip))&&(i->netmaskBits() != ip.netmaskBits())) {
+			if (___removeIp(_dev,*i)) {
+				_ips.erase(i);
+				break;
+			} else {
+				LOG("WARNING: failed to remove old IP/netmask %s to replace with %s",i->toString().c_str(),ip.toString().c_str());
+			}
 		}
 	}
 
@@ -530,8 +533,12 @@ bool EthernetTap::addIP(const InetAddress &ip)
 bool EthernetTap::removeIP(const InetAddress &ip)
 {
 	Mutex::Lock _l(_ips_m);
-	if (_ips.count(ip) > 0)
-		return ___removeIp(_dev,_ips,ip);
+	if (_ips.count(ip) > 0) {
+		if (___removeIp(_dev,ip)) {
+			_ips.erase(ip);
+			return true;
+		}
+	}
 	return false;
 }
 
diff --git a/node/MulticastGroup.hpp b/node/MulticastGroup.hpp
index ac7de481..3c654ea2 100644
--- a/node/MulticastGroup.hpp
+++ b/node/MulticastGroup.hpp
@@ -36,7 +36,7 @@
 namespace ZeroTier {
 
 /**
- * A multicast group composed of a multicast MAC and a 64-bit ADI field
+ * A multicast group composed of a multicast MAC and a 32-bit ADI field
  *
  * ADI stands for additional distinguishing information. ADI is primarily for
  * adding additional information to broadcast (ff:ff:ff:ff:ff:ff) memberships,
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 3fade674..f8f340b8 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -333,40 +333,40 @@ public:
 		VERB_NOP = 0,
 
 		/* Announcement of a node's existence:
-		 *	 <[1] protocol version>
+		 *   <[1] protocol version>
 		 *   <[1] software major version>
 		 *   <[1] software minor version>
 		 *   <[2] software revision>
-		 *	 <[8] timestamp (ms since epoch)>
-		 *	 <[...] binary serialized identity (see Identity)>
+		 *   <[8] timestamp (ms since epoch)>
+		 *   <[...] binary serialized identity (see Identity)>
 		 *
 		 * OK payload:
-		 *	 <[8] timestamp (echoed from original HELLO)>
+		 *   <[8] timestamp (echoed from original HELLO)>
 		 *
 		 * ERROR has no payload.
 		 */
 		VERB_HELLO = 1,
 
 		/* Error response:
-		 *	 <[1] in-re verb>
-		 *	 <[8] in-re packet ID>
-		 *	 <[1] error code>
-		 *	 <[...] error-dependent payload>
+		 *   <[1] in-re verb>
+		 *   <[8] in-re packet ID>
+		 *   <[1] error code>
+		 *   <[...] error-dependent payload>
 		 */
 		VERB_ERROR = 2,
 
 		/* Success response:
-		 *	 <[1] in-re verb>
-		 *	 <[8] in-re packet ID>
-		 *	 <[...] request-specific payload>
+		 *   <[1] in-re verb>
+		 *   <[8] in-re packet ID>
+		 *   <[...] request-specific payload>
 		 */
 		VERB_OK = 3,
 
 		/* Query an identity by address:
-		 *	 <[5] address to look up>
+		 *   <[5] address to look up>
 		 *
 		 * OK response payload:
-		 *	 <[...] binary serialized identity>
+		 *   <[...] binary serialized identity>
 		 *
 		 * Error payload will be address queried.
 		 */
@@ -400,8 +400,8 @@ public:
 
 		/* A ZT-to-ZT unicast ethernet frame:
 		 *   <[8] 64-bit network ID>
-		 *	 <[2] 16-bit ethertype>
-		 *	 <[...] ethernet payload>
+		 *   <[2] 16-bit ethertype>
+		 *   <[...] ethernet payload>
 		 *
 		 * MAC addresses are derived from the packet's source and destination
 		 * ZeroTier addresses. ZeroTier does not support VLANs or other extensions