From 0d1eaba89081765aaee7f6857987c52c1bd6224d Mon Sep 17 00:00:00 2001 From: thunder2 Date: Sun, 13 Jun 2010 12:26:23 +0000 Subject: [PATCH] p3StatusService::getStatusQueue - memory leak -> "RsItem* item" was not freed, when "dynamic_cast" failed - optimized return of std::list as parameter and not as return. return will copy the list and its not necessary p3StatusService::getStatus - memory leak -> items in "std::list status_items" was not freed - potential crash, when receiving a status for an unknown peer p3Peers::getPeerDetails - optimized - call to "AuthSSL::getAuthSSL()->OwnId()" only once - optimized - add ip addresses directly to "d.ipAddressList" PeersDialog::insertPeers - fixed possible crash, when ssl child has disappeared and was removed from tree, there was a missing continue git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@3125 b45a01b8-16f6-495d-af2f-9b41ad6348cc --- libretroshare/src/pqi/authgpg.cc | 24 ++++---- libretroshare/src/rsserver/p3peers.cc | 10 ++-- libretroshare/src/services/p3statusservice.cc | 59 +++++++++---------- libretroshare/src/services/p3statusservice.h | 2 +- retroshare-gui/src/gui/PeersDialog.cpp | 9 ++- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/libretroshare/src/pqi/authgpg.cc b/libretroshare/src/pqi/authgpg.cc index f6f17062a..b00918569 100644 --- a/libretroshare/src/pqi/authgpg.cc +++ b/libretroshare/src/pqi/authgpg.cc @@ -996,17 +996,19 @@ bool AuthGPG::getGPGDetails(std::string id, RsPeerDetails &d) certmap::iterator it; if (mKeyList.end() != (it = mKeyList.find(id))) { - d.id = it->second.id; //keep, it but can be bug gen - d.gpg_id = it->second.id; - d.name = it->second.name; - d.email = it->second.email; - d.trustLvl = it->second.trustLvl; - d.validLvl = it->second.validLvl; - d.ownsign = it->second.ownsign; - d.gpgSigners = it->second.signers; - d.fpr = it->second.fpr; + gpgcert &cert = it->second; - d.accept_connection = it->second.accept_connection; + d.id = cert.id; //keep, it but can be bug gen + d.gpg_id = cert.id; + d.name = cert.name; + d.email = cert.email; + d.trustLvl = cert.trustLvl; + d.validLvl = cert.validLvl; + d.ownsign = cert.ownsign; + d.gpgSigners = cert.signers; + d.fpr = cert.fpr; + + d.accept_connection = cert.accept_connection; //did the peer signed me ? d.hasSignedMe = false; @@ -1019,7 +1021,7 @@ bool AuthGPG::getGPGDetails(std::string id, RsPeerDetails &d) } #ifdef GPG_DEBUG - std::cerr << "AuthGPG::getPGPDetails() Name : " << it->second.name << std::endl; + std::cerr << "AuthGPG::getPGPDetails() Name : " << cert.name << std::endl; #endif return true; } diff --git a/libretroshare/src/rsserver/p3peers.cc b/libretroshare/src/rsserver/p3peers.cc index ce2ceb87d..ddade0d15 100644 --- a/libretroshare/src/rsserver/p3peers.cc +++ b/libretroshare/src/rsserver/p3peers.cc @@ -276,8 +276,9 @@ bool p3Peers::getPeerDetails(std::string id, RsPeerDetails &d) std::cerr << "p3Peers::getPeerDetails() called for id : " << id << std::endl; #endif //first, check if it's a gpg or a ssl id. + std::string sOwnId = AuthSSL::getAuthSSL()->OwnId(); peerConnectState pcs; - if (id != AuthSSL::getAuthSSL()->OwnId() && !mConnMgr->getFriendNetStatus(id, pcs)) { + if (id != sOwnId && !mConnMgr->getFriendNetStatus(id, pcs)) { //assume is not SSL, because every ssl_id has got a friend correspondance in mConnMgr #ifdef P3PEERS_DEBUG std::cerr << "p3Peers::getPeerDetails() got a gpg id and is returning GPG details only for id : " << id << std::endl; @@ -289,7 +290,7 @@ bool p3Peers::getPeerDetails(std::string id, RsPeerDetails &d) std::cerr << "p3Peers::getPeerDetails() got a SSL id and is returning SSL and GPG details for id : " << id << std::endl; #endif - if (id == AuthSSL::getAuthSSL()->OwnId()) { + if (id == sOwnId) { mConnMgr->getOwnNetStatus(pcs); pcs.gpg_id = AuthGPG::getAuthGPG()->getGPGOwnId(); } @@ -322,14 +323,13 @@ bool p3Peers::getPeerDetails(std::string id, RsPeerDetails &d) d.dyndns = pcs.dyndns; d.lastConnect = pcs.lastcontact; d.connectPeriod = 0; - std::list ipAddressList; std::list pcsList = pcs.getIpAddressList(); + d.ipAddressList.clear(); for (std::list::iterator ipListIt = pcsList.begin(); ipListIt!=(pcsList.end()); ipListIt++) { std::ostringstream toto; toto << ntohs(ipListIt->ipAddr.sin_port) << " " << (time(NULL) - ipListIt->seenTime) << " sec"; - ipAddressList.push_back(std::string(inet_ntoa(ipListIt->ipAddr.sin_addr)) + ":" + toto.str()); + d.ipAddressList.push_back(std::string(inet_ntoa(ipListIt->ipAddr.sin_addr)) + ":" + toto.str()); } - d.ipAddressList = ipAddressList; /* Translate */ diff --git a/libretroshare/src/services/p3statusservice.cc b/libretroshare/src/services/p3statusservice.cc index 8ad0bed60..4c5ddbd6d 100644 --- a/libretroshare/src/services/p3statusservice.cc +++ b/libretroshare/src/services/p3statusservice.cc @@ -65,8 +65,8 @@ bool p3StatusService::getStatus(std::list& statusInfo) statusInfo.clear(); - std::list status_items = getStatusQueue(); - std::list::iterator rit; + std::list status_items; + getStatusQueue(status_items); std::map::iterator mit; std::list peers, peersOnline; std::list::iterator pit, pit_online; @@ -98,29 +98,24 @@ bool p3StatusService::getStatus(std::list& statusInfo) } // now note members who have sent specific status updates - for(rit = status_items.begin(); rit != status_items.end(); rit++){ - - RsStatusItem* si = dynamic_cast(*rit); - - if(si == NULL){ - std::cerr << "p3Status::getStatus() " << "Failed to cast Item \n" << std::endl; - } - + while (status_items.size()){ + RsStatusItem* si = status_items.front(); + status_items.pop_front(); mit = mStatusInfoMap.find(si->PeerId()); - + if(mit != mStatusInfoMap.end()){ + mit->second.id = si->PeerId(); + mit->second.status = si->status; + mit->second.time_stamp = si->sendTime; #ifdef STATUS_DEBUG - if(mit == mStatusInfoMap.end()){ + } else { std::cerr << "p3GetStatus() " << "Could not find Peer" << si->PeerId(); std::cerr << std::endl; - } #endif + } - mit->second.id = si->PeerId(); - mit->second.status = si->status; - mit->second.time_stamp = si->sendTime; - + delete (si); } // then fill up statusInfo list with this information @@ -190,31 +185,31 @@ bool p3StatusService::statusAvailable(){ /******************************/ -std::list p3StatusService::getStatusQueue(){ - - +void p3StatusService::getStatusQueue(std::list &ilist) +{ time_t time_now = time(NULL); RsItem* item; - std::list ilist; while(NULL != (item = recvItem())){ RsStatusItem* status_item = dynamic_cast(item); - if(status_item != NULL){ -#ifdef STATUS_DEBUG - std::cerr << "p3StatusService::getStatusQueue()" << std::endl; - std::cerr << "PeerId : " << status_item->PeerId() << std::endl; - std::cerr << "Status: " << status_item->status << std::endl; - std::cerr << "Got status Item" << std::endl; -#endif - status_item->recvTime = time_now; - ilist.push_back(status_item); + if(status_item == NULL) { + std::cerr << "p3Status::getStatusQueue() " << "Failed to cast Item \n" << std::endl; + delete (item); + continue; } - } - return ilist; +#ifdef STATUS_DEBUG + std::cerr << "p3StatusService::getStatusQueue()" << std::endl; + std::cerr << "PeerId : " << status_item->PeerId() << std::endl; + std::cerr << "Status: " << status_item->status << std::endl; + std::cerr << "Got status Item" << std::endl; +#endif + status_item->recvTime = time_now; + ilist.push_back(status_item); + } } /* p3Config */ diff --git a/libretroshare/src/services/p3statusservice.h b/libretroshare/src/services/p3statusservice.h index 41f180444..224c126e0 100644 --- a/libretroshare/src/services/p3statusservice.h +++ b/libretroshare/src/services/p3statusservice.h @@ -83,7 +83,7 @@ virtual bool loadList(std::list load); private: -virtual std::list getStatusQueue(); +virtual void getStatusQueue(std::list &ilist); p3ConnectMgr *mConnMgr; diff --git a/retroshare-gui/src/gui/PeersDialog.cpp b/retroshare-gui/src/gui/PeersDialog.cpp index 8ec7ceec1..3de6682ab 100644 --- a/retroshare-gui/src/gui/PeersDialog.cpp +++ b/retroshare-gui/src/gui/PeersDialog.cpp @@ -512,9 +512,11 @@ void PeersDialog::insertPeers() continue; } + bool bNew = false; if (gpg_item == NULL) { gpg_item = new MyTreeWidgetItem(peertreeWidget, 0); //set type to 0 for custom popup menu gpg_item->setChildIndicatorPolicy(QTreeWidgetItem::DontShowIndicatorWhenChildless); + bNew = true; } gpg_item -> setText(COLUMN_NAME, QString::fromStdString(detail.name)); @@ -565,6 +567,7 @@ void PeersDialog::insertPeers() if (sslItem) { gpg_item->removeChild(sslItem); } + continue; } if (newChild) { @@ -733,8 +736,10 @@ void PeersDialog::insertPeers() } } - /* add gpg item to the list. If item is already in the list, it won't be duplicated thanks to Qt */ - peertreeWidget->addTopLevelItem(gpg_item); + if (bNew) { + /* add gpg item to the list */ + peertreeWidget->addTopLevelItem(gpg_item); + } } }