From 6eaf20112098f40c2bda45eebd57dbd110bacebb Mon Sep 17 00:00:00 2001 From: joss17 Date: Fri, 30 Oct 2009 00:39:43 +0000 Subject: [PATCH] improve the async action and getStateVariable git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@1764 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libretroshare/src/upnp/UPnPBase.cpp | 99 ++++++++++------ libretroshare/src/upnp/upnphandler.cc | 162 +++++++++++++------------- 2 files changed, 144 insertions(+), 117 deletions(-) diff --git a/libretroshare/src/upnp/UPnPBase.cpp b/libretroshare/src/upnp/UPnPBase.cpp index 25b65bc4c..724be6749 100644 --- a/libretroshare/src/upnp/UPnPBase.cpp +++ b/libretroshare/src/upnp/UPnPBase.cpp @@ -729,37 +729,48 @@ bool CUPnPService::Execute( const std::string CUPnPService::GetStateVariable( const std::string &stateVariableName) { - //this getvar is just to make an event happening - DOMString StVarVal; - int ret = UpnpGetServiceVarStatus( - m_UPnPControlPoint.GetUPnPClientHandle(), - GetAbsControlURL().c_str(), - stateVariableName.c_str(), - &StVarVal); - if (StVarVal != NULL) { - std::string varValue = std::string(StVarVal); - std::cerr << "GetStateVariable varValue returned by UpnpGetServiceVarStatus : " << varValue << std::endl; - return varValue; + std::map::iterator it; + it = propertyMap.find(stateVariableName); + if (it != propertyMap.end()) { + std::cerr << "GetStateVariable(" << stateVariableName << ") = "; + std::cerr << (*it).second << std::endl; + return (*it).second; } else { - //use event to get state variable - std::cerr << "GetStateVariable pausing in case of an UPnP event incomming."; - time_t begin_time = time(NULL); - while (true) { - if (time(NULL) - begin_time > 7) { - break; + //property map is not populated with the specified value. + //we will try to get it with an event + + //this getvar is just to make the event happening + DOMString StVarVal; + UpnpGetServiceVarStatus( + m_UPnPControlPoint.GetUPnPClientHandle(), + GetAbsControlURL().c_str(), + stateVariableName.c_str(), + &StVarVal); + if (StVarVal != NULL) { + std::string varValue = std::string(StVarVal); + std::cerr << "GetStateVariable varValue returned by UpnpGetServiceVarStatus : " << varValue << std::endl; + return varValue; + } else { + //use event to get state variable + std::cerr << "GetStateVariable pausing in case of an UPnP event incomming."; + time_t begin_time = time(NULL); + while (true) { + if (time(NULL) - begin_time > 7) { + break; + } } } - std::cerr << "GetStateVariable pause finished."; - std::cerr << "GetStateVariable(" << stateVariableName << ") = "; - std::map::iterator it; + //propertyMap should be populated by nom it = propertyMap.find(stateVariableName); - if (it == propertyMap.end()) { - std::cerr << "Empty String" << std::endl; - return stdEmptyString; - } else { + if (it != propertyMap.end()) { + std::cerr << "GetStateVariable(" << stateVariableName << ") = "; std::cerr << (*it).second << std::endl; return (*it).second; + } else { + std::cerr << "GetStateVariable(" << stateVariableName << ") = "; + std::cerr << "Empty String" << std::endl; + return stdEmptyString; } } } @@ -929,6 +940,11 @@ m_WanService(NULL) } + //clean the PortMappingNumberOfEntries as it is erroneus on the first event with the french neufbox + if (WanServiceDetected()) { + m_WanService->propertyMap.erase("PortMappingNumberOfEntries"); + } + std::cerr << "CUPnPControlPoint Constructor finished" << std::endl; return; @@ -976,6 +992,14 @@ bool CUPnPControlPoint::AddPortMappings( bool ok = false; // Check the number of port mappings before + //have a little break in case we just modified the variable, so we have to wait for an event + std::cerr << "GetStateVariable pausing in case of an UPnP event incomming."; + time_t begin_time = time(NULL); + while (true) { + if (time(NULL) - begin_time > 7) { + break; + } + } std::istringstream OldPortMappingNumberOfEntries( m_WanService->GetStateVariable( "PortMappingNumberOfEntries")); @@ -994,15 +1018,16 @@ bool CUPnPControlPoint::AddPortMappings( PrivateAddPortMapping(upnpPortMapping[i]); } } - - // Debug only -#ifdef UPNP_DEBUG - std::cerr << "CUPnPControlPoint::AddPortMappings: " - "m_ActivePortMappingsMap.size() == " << - m_ActivePortMappingsMap.size() << std::endl; -#endif - // Not very good, must find a better test + // Not very good, must find a better test : check the new number of port entries + //have a little break in case we just modified the variable, so we have to wait for an event + std::cerr << "GetStateVariable pausing in case of an UPnP event incomming."; + begin_time = time(NULL); + while (true) { + if (time(NULL) - begin_time > 7) { + break; + } + } std::istringstream NewPortMappingNumberOfEntries( m_WanService->GetStateVariable( "PortMappingNumberOfEntries")); @@ -1024,10 +1049,16 @@ std::string CUPnPControlPoint::getExternalAddress() "WAN Service not detected." << std::endl; return false; } - PrivateGetExternalIpAdress(); std::string result = m_WanService->GetStateVariable("NewExternalIPAddress"); + std::cerr << " m_WanService->GetStateVariable(NewExternalIPAddress) = " << result << std::endl; if (result == "") { - result = m_WanService->GetStateVariable("ExternalIPAddress"); + PrivateGetExternalIpAdress(); + result = m_WanService->GetStateVariable("NewExternalIPAddress"); + std::cerr << " m_WanService->GetStateVariable(NewExternalIPAddress) = " << result << std::endl; + if (result == "") { + result = m_WanService->GetStateVariable("ExternalIPAddress"); + std::cerr << " m_WanService->GetStateVariable(ExternalIPAddress) = " << result << std::endl; + } } return result; } diff --git a/libretroshare/src/upnp/upnphandler.cc b/libretroshare/src/upnp/upnphandler.cc index 268415921..d861a6d31 100644 --- a/libretroshare/src/upnp/upnphandler.cc +++ b/libretroshare/src/upnp/upnphandler.cc @@ -100,8 +100,6 @@ bool upnphandler::background_setup_upnp(bool start, bool stop) bool upnphandler::start_upnp() { - RsStackMutex stack(dataMtx); /* LOCK STACK MUTEX */ - if (!(upnpState >= RS_UPNP_S_READY)) { std::cerr << "upnphandler::start_upnp() Not Ready" << std::endl; @@ -111,94 +109,79 @@ bool upnphandler::start_upnp() char eprot1[] = "TCP"; char eprot2[] = "UDP"; - /* if we're to load -> load */ - /* select external ports */ - eport_curr = eport; - if (!eport_curr) + struct sockaddr_in localAddr; { - /* use local port if eport is zero */ - eport_curr = iport; - std::cerr << "Using LocalPort for extPort!"; - std::cerr << std::endl; + RsStackMutex stack(dataMtx); /* LOCK STACK MUTEX */ + + /* if we're to load -> load */ + /* select external ports */ + eport_curr = eport; + if (!eport_curr) + { + /* use local port if eport is zero */ + eport_curr = iport; + std::cerr << "Using LocalPort for extPort!"; + std::cerr << std::endl; + } + + if (!eport_curr) + { + std::cerr << "Invalid eport ... "; + std::cerr << std::endl; + return false; + } + + + /* our port */ + char in_addr[256]; + char in_port1[256]; + char eport1[256]; + + upnp_iaddr.sin_port = htons(iport); + localAddr = upnp_iaddr; + uint32_t linaddr = ntohl(localAddr.sin_addr.s_addr); + + snprintf(in_port1, 256, "%d", ntohs(localAddr.sin_port)); + snprintf(in_addr, 256, "%d.%d.%d.%d", + ((linaddr >> 24) & 0xff), + ((linaddr >> 16) & 0xff), + ((linaddr >> 8) & 0xff), + ((linaddr >> 0) & 0xff)); + + snprintf(eport1, 256, "%d", eport_curr); + + std::cerr << "Attempting Redirection: InAddr: " << in_addr; + std::cerr << " InPort: " << in_port1; + std::cerr << " ePort: " << eport1; + std::cerr << " eProt: " << eprot1; + std::cerr << std::endl; } - if (!eport_curr) - { - std::cerr << "Invalid eport ... "; - std::cerr << std::endl; - return false; - } - - - /* our port */ - char in_addr[256]; - char in_port1[256]; - char eport1[256]; - - upnp_iaddr.sin_port = htons(iport); - struct sockaddr_in localAddr = upnp_iaddr; - uint32_t linaddr = ntohl(localAddr.sin_addr.s_addr); - - snprintf(in_port1, 256, "%d", ntohs(localAddr.sin_port)); - snprintf(in_addr, 256, "%d.%d.%d.%d", - ((linaddr >> 24) & 0xff), - ((linaddr >> 16) & 0xff), - ((linaddr >> 8) & 0xff), - ((linaddr >> 0) & 0xff)); - - snprintf(eport1, 256, "%d", eport_curr); - - std::cerr << "Attempting Redirection: InAddr: " << in_addr; - std::cerr << " InPort: " << in_port1; - std::cerr << " ePort: " << eport1; - std::cerr << " eProt: " << eprot1; - std::cerr << std::endl; - //build port mapping config std::vector upnpPortMapping1; CUPnPPortMapping cUPnPPortMapping1 = CUPnPPortMapping(eport_curr, ntohs(localAddr.sin_port), "TCP", true, "tcp retroshare redirection"); upnpPortMapping1.push_back(cUPnPPortMapping1); bool res = cUPnPControlPoint->AddPortMappings(upnpPortMapping1); - if (res) { - upnpState = RS_UPNP_S_ACTIVE; - } else { - upnpState = RS_UPNP_S_TCP_FAILED; - std::vector upnpPortMapping2; - CUPnPPortMapping cUPnPPortMapping2 = CUPnPPortMapping(eport_curr, ntohs(localAddr.sin_port), "UDP", true, "udp retroshare redirection"); - upnpPortMapping2.push_back(cUPnPPortMapping2); - bool res2 = cUPnPControlPoint->AddPortMappings(upnpPortMapping2); - if (res2) { - upnpState = RS_UPNP_S_ACTIVE; - } else { - //upnpState = RS_UPNP_S_ACTIVE; - upnpState = RS_UPNP_S_UDP_FAILED; - } - } + std::vector upnpPortMapping2; + CUPnPPortMapping cUPnPPortMapping2 = CUPnPPortMapping(eport_curr, ntohs(localAddr.sin_port), "UDP", true, "udp retroshare redirection"); + upnpPortMapping2.push_back(cUPnPPortMapping2); + bool res2 = cUPnPControlPoint->AddPortMappings(upnpPortMapping2); - /* now store the external address */ - std::string externalAdress = cUPnPControlPoint->getExternalAddress(); - sockaddr_clear(&upnp_eaddr); + struct sockaddr_in extAddr; + bool extAddrResult = getExternalAddress(extAddr); - if(!externalAdress.empty()) { - const char* externalIPAddress = externalAdress.c_str(); + RsStackMutex stack(dataMtx); /* LOCK STACK MUTEX */ - std::cerr << "Stored External address: " << externalIPAddress; - std::cerr << ":" << eport_curr; - std::cerr << std::endl; + if (extAddrResult && (res || res2)) { + upnpState = RS_UPNP_S_ACTIVE; + } else { + upnpState = RS_UPNP_S_UDP_FAILED; + } - inet_aton(externalIPAddress, &(upnp_eaddr.sin_addr)); - upnp_eaddr.sin_family = AF_INET; - upnp_eaddr.sin_port = htons(eport_curr); + toStart = false; } - else - { - std::cerr << "FAILED To get external Address"; - std::cerr << std::endl; - } - - toStart = false; return (upnpState == RS_UPNP_S_ACTIVE); @@ -414,15 +397,28 @@ bool upnphandler::getInternalAddress(struct sockaddr_in &addr) bool upnphandler::getExternalAddress(struct sockaddr_in &addr) { -// std::cerr << "UPnPHandler::getExternalAddress() pre Lock!" << std::endl; - dataMtx.lock(); /*** LOCK MUTEX ***/ -// std::cerr << "UPnPHandler::getExternalAddress() postLock!" << std::endl; + std::string externalAdress = cUPnPControlPoint->getExternalAddress(); - std::cerr << "UPnPHandler::getExternalAddress()" << std::endl; - addr = upnp_eaddr; - bool valid = (upnpState == RS_UPNP_S_ACTIVE); + if(!externalAdress.empty()) + { + const char* externalIPAddress = externalAdress.c_str(); - dataMtx.unlock(); /*** UNLOCK MUTEX ***/ + std::cerr << "Stored External address: " << externalIPAddress; + std::cerr << ":" << eport_curr; + std::cerr << std::endl; - return valid; + dataMtx.lock(); /*** LOCK MUTEX ***/ + sockaddr_clear(&upnp_eaddr); + inet_aton(externalIPAddress, &(upnp_eaddr.sin_addr)); + upnp_eaddr.sin_family = AF_INET; + upnp_eaddr.sin_port = htons(eport_curr); + dataMtx.unlock(); /*** UNLOCK MUTEX ***/ + + addr = upnp_eaddr; + return true; + } + else + { + return false; + } }