Skip to content

Commit

Permalink
Merge pull request #3112 from randaz81/fix_broker
Browse files Browse the repository at this point in the history 
fixed dangling pointer using std::string
  • Loading branch information
@randaz81
randaz81 authored Aug 30, 2024
2 parents c5e4d05 + a62e65e commit b26883d
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 110 deletions.
17 changes: 7 additions & 10 deletions src/libYARP_manager/src/yarp/manager/broker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,13 @@ class Broker {
virtual bool start() = 0;
virtual bool stop() = 0;
virtual bool kill() = 0;
virtual bool connect(const char* from, const char* to,
const char* carrier, bool persist=false) = 0;
virtual bool disconnect(const char* from, const char* to,
const char* carrier) = 0;
virtual bool connect(const std::string& from, const std::string& to, const std::string& carrier, bool persist = false) = 0;
virtual bool disconnect(const std::string& from, const std::string& to, const std::string& carrier) = 0;
virtual int running() = 0; // 0 if is not running and 1 if is running; otherwise -1.
virtual bool exists(const char* port) = 0;
virtual const char* requestRpc(const char* szport, const char* request, double timeout=0.0) = 0;
virtual bool connected(const char* from, const char* to,
const char* carrier) = 0;
virtual const char* error() = 0;
virtual bool exists(const std::string& port) = 0;
virtual std::string requestRpc(const std::string& szport, const std::string& request, double timeout = 0.0) = 0;
virtual bool connected(const std::string& from, const std::string& to, const std::string& carrier) = 0;
virtual std::string error() = 0;
virtual bool initialized() = 0;
virtual bool attachStdout() = 0;
virtual void detachStdout() = 0;
Expand All @@ -62,7 +59,7 @@ class Broker {
bool hasWatchDog() { return bWithWatchDog; }
void setDisplay(const char* szDisplay) { if(szDisplay) { strDisplay = szDisplay; } }

const char* getDisplay() const {return strDisplay.c_str(); }
std::string getDisplay() const {return strDisplay; }
protected:
unsigned int UNIQUEID;
BrokerEventSink* eventSink;
Expand Down
16 changes: 8 additions & 8 deletions src/libYARP_manager/src/yarp/manager/execstate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ bool Ready::checkResources(bool silent)
}
// check the rpc request/reply if required
if(strlen((*itr).getRequest()) != 0) {
const char* reply = executable->getBroker()->requestRpc((*itr).getPort(),
std::string reply = executable->getBroker()->requestRpc((*itr).getPort(),
(*itr).getRequest(),
(*itr).getTimeout());
if(reply == nullptr) {
if(reply.empty()) {
allOK = false;
OSTRINGSTREAM msg;
msg<<"cannot request resource "<<(*itr).getPort()<<" for "<<(*itr).getRequest();
Expand All @@ -142,7 +142,7 @@ bool Ready::checkResources(bool silent)
}
}

if(!compareString(reply, (*itr).getReply())) {
if (!compareString(reply.c_str(), (*itr).getReply())) {
allOK = false;
OSTRINGSTREAM msg;
msg<<"waiting for the expected reply from resource "<<(*itr).getPort();
Expand Down Expand Up @@ -229,7 +229,7 @@ void Ready::startModule()
{
OSTRINGSTREAM msg;
msg<<"cannot run "<<executable->getCommand()<<" on "<<executable->getHost();
if (executable->getBroker()->error()) {
if (!executable->getBroker()->error().empty()) {
msg << " : " << executable->getBroker()->error();
}
logger->addError(msg);
Expand Down Expand Up @@ -305,7 +305,7 @@ void Connecting::connectAllPorts()
{
OSTRINGSTREAM msg;
msg<<"cannot connect "<<(*itr).from() <<" to "<<(*itr).to();
if (executable->getBroker()->error()) {
if (!executable->getBroker()->error().empty()) {
msg << " : " << executable->getBroker()->error();
}
logger->addError(msg);
Expand Down Expand Up @@ -418,7 +418,7 @@ void Dying::stopModule()
{
OSTRINGSTREAM msg;
msg<<"cannot stop "<<executable->getCommand()<<" on "<<executable->getHost();
if (executable->getBroker()->error()) {
if (!executable->getBroker()->error().empty()) {
msg << " : " << executable->getBroker()->error();
}
logger->addError(msg);
Expand All @@ -439,7 +439,7 @@ void Dying::killModule()
{
OSTRINGSTREAM msg;
msg<<"cannot kill "<<executable->getCommand()<<" on "<<executable->getHost();
if (executable->getBroker()->error()) {
if (!executable->getBroker()->error().empty()) {
msg << " : " << executable->getBroker()->error();
}
logger->addError(msg);
Expand Down Expand Up @@ -467,7 +467,7 @@ void Dying::disconnectAllPorts()
{
OSTRINGSTREAM msg;
msg<<"cannot disconnect "<<(*itr).from() <<" to "<<(*itr).to();
if (executable->getBroker()->error()) {
if (!executable->getBroker()->error().empty()) {
msg << " : " << executable->getBroker()->error();
}
logger->addError(msg);
Expand Down
2 changes: 1 addition & 1 deletion src/libYARP_manager/src/yarp/manager/executable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ bool Executable::initialize()
{
OSTRINGSTREAM msg;
msg<<"cannot initialize broker. : ";
if (broker->error()) {
if (!broker->error().empty()) {
msg << broker->error();
}
logger->addError(msg);
Expand Down
35 changes: 17 additions & 18 deletions src/libYARP_manager/src/yarp/manager/localbroker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,17 +355,16 @@ int LocalBroker::running()
/**
* connection broker
*/
bool LocalBroker::connect(const char* from, const char* to,
const char* carrier, bool persist)
bool LocalBroker::connect(const std::string& from, const std::string& to, const std::string& carrier, bool persist)
{

if(!from)
if(from.empty())
{
strError = "no source port is introduced.";
return false;
}

if(!to)
if (to.empty())
{
strError = "no destination port is introduced.";
return false;
Expand Down Expand Up @@ -395,16 +394,16 @@ bool LocalBroker::connect(const char* from, const char* to,
return true;
}

bool LocalBroker::disconnect(const char* from, const char* to, const char *carrier)
bool LocalBroker::disconnect(const std::string& from, const std::string& to, const std::string& carrier)
{

if(!from)
if (from.empty())
{
strError = "no source port is introduced.";
return false;
}

if(!to)
if (to.empty())
{
strError = "no destination port is introduced.";
return false;
Expand Down Expand Up @@ -439,27 +438,27 @@ bool LocalBroker::disconnect(const char* from, const char* to, const char *carri

}

bool LocalBroker::exists(const char* port)
bool LocalBroker::exists(const std::string& port)
{
return NetworkBase::exists(port);
}


const char* LocalBroker::requestRpc(const char* szport, const char* request, double timeout)
std::string LocalBroker::requestRpc(const std::string& szport, const std::string& request, double timeout)
{
if ((szport == nullptr) || (request == nullptr)) {
return nullptr;
if (szport.empty() || request.empty()) {
return {};
}

if (!exists(szport)) {
return nullptr;
return {};
}

// opening the port
yarp::os::Port port;
port.setTimeout((float)((timeout>0.0) ? timeout : CONNECTION_TIMEOUT));
if (!port.open("...")) {
return nullptr;
return {};
}

ContactStyle style;
Expand All @@ -476,7 +475,7 @@ const char* LocalBroker::requestRpc(const char* szport, const char* request, dou

if(!ret) {
port.close();
return nullptr;
return {};
}

Bottle msg, response;
Expand All @@ -485,14 +484,14 @@ const char* LocalBroker::requestRpc(const char* szport, const char* request, dou
NetworkBase::disconnect(port.getName(), szport);
if(!response.size() || !ret) {
port.close();
return nullptr;
return {};
}

port.close();
return response.toString().c_str();
}

bool LocalBroker::connected(const char* from, const char* to, const char* carrier)
bool LocalBroker::connected(const std::string& from, const std::string& to, const std::string& carrier)
{
if (!exists(from) || !exists(to)) {
return false;
Expand All @@ -501,9 +500,9 @@ bool LocalBroker::connected(const char* from, const char* to, const char* carrie
}


const char* LocalBroker::error()
std::string LocalBroker::error()
{
return strError.c_str();
return strError;
}

bool LocalBroker::attachStdout()
Expand Down
15 changes: 6 additions & 9 deletions src/libYARP_manager/src/yarp/manager/localbroker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,13 @@ class LocalBroker: public Broker, public yarp::os::Thread {
bool start() override;
bool stop() override;
bool kill() override;
bool connect(const char* from, const char* to,
const char* carrier, bool persist=false) override;
bool disconnect(const char* from, const char* to,
const char *carrier) override;
bool connect(const std::string& from, const std::string& to, const std::string& carrier, bool persist = false) override;
bool disconnect(const std::string& from, const std::string& to, const std::string& carrier) override;
int running() override;
bool exists(const char* port) override;
const char* requestRpc(const char* szport, const char* request, double timeout) override;
bool connected(const char* from, const char* to,
const char* carrier) override;
const char* error() override;
bool exists(const std::string& port) override;
std::string requestRpc(const std::string& szport, const std::string& request, double timeout) override;
bool connected(const std::string& from, const std::string& to, const std::string& carrier) override;
std::string error() override;
bool initialized() override { return bInitialized;}
bool attachStdout() override;
void detachStdout() override;
Expand Down
4 changes: 2 additions & 2 deletions src/libYARP_manager/src/yarp/manager/scriptbroker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ bool ScriptLocalBroker::init(const char* szcmd, const char* szparam,
}


bool ScriptYarprunBroker::whichFile(const char* server, const char* filename, std::string& filenameWithPath)
bool ScriptYarprunBroker::whichFile(const std::string& server, const std::string& filename, std::string& filenameWithPath)
{
if (!strlen(server)) {
if (server.empty()) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libYARP_manager/src/yarp/manager/scriptbroker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class ScriptYarprunBroker: public YarpBroker
const char* szhost, const char* szstdio,
const char* szworkdir, const char* szenv) override;
private:
bool whichFile(const char* server, const char* filename, std::string& filenameWithPath);
std::string script;
bool whichFile(const std::string& server, const std::string& filename, std::string& filenameWithPath);
std::string script;
};

} // namespace yarp::manager
Expand Down
Loading

0 comments on commit b26883d

Please sign in to comment.