summaryrefslogtreecommitdiff
path: root/cpp
diff options
context:
space:
mode:
Diffstat (limited to 'cpp')
-rw-r--r--cpp/src/IcePack/ActivatorI.cpp5
-rw-r--r--cpp/src/IcePack/ActivatorI.h2
-rw-r--r--cpp/src/IcePack/ComponentDeployer.cpp20
-rw-r--r--cpp/src/IcePack/ComponentDeployer.h12
-rw-r--r--cpp/src/IcePack/LocatorI.h2
-rw-r--r--cpp/src/IcePack/LocatorRegistryI.h2
-rw-r--r--cpp/src/IcePack/Parser.cpp57
-rw-r--r--cpp/src/IcePack/Server.cpp2
-rw-r--r--cpp/src/IcePack/ServerDeployer.cpp17
-rw-r--r--cpp/src/IcePack/ServerManagerI.cpp5
-rw-r--r--cpp/src/IcePack/ServerManagerI.h2
-rw-r--r--cpp/src/IcePack/ServiceDeployer.cpp2
-rw-r--r--cpp/src/IcePack/ServiceDeployer.h1
13 files changed, 77 insertions, 52 deletions
diff --git a/cpp/src/IcePack/ActivatorI.cpp b/cpp/src/IcePack/ActivatorI.cpp
index dd11db49854..f8e7a7f66b2 100644
--- a/cpp/src/IcePack/ActivatorI.cpp
+++ b/cpp/src/IcePack/ActivatorI.cpp
@@ -158,6 +158,11 @@ IcePack::ActivatorI::activate(const ServerPrx& server, const ::Ice::Current&)
// Ice logger could be changed to flush the stream and not use
// automatic flushing.
//
+
+// TODO: ML: Can you be more specific? Sending messages through a pipe
+// is one of the most basic unix mechanisms. I don't see how this
+// could not work.
+
// if(fds[1] != STDERR_FILENO)
// {
// if(dup2(fds[1], STDERR_FILENO) != STDERR_FILENO)
diff --git a/cpp/src/IcePack/ActivatorI.h b/cpp/src/IcePack/ActivatorI.h
index bc5064d02c2..ad95a039afc 100644
--- a/cpp/src/IcePack/ActivatorI.h
+++ b/cpp/src/IcePack/ActivatorI.h
@@ -15,6 +15,8 @@
#include <IcePack/Activator.h>
#include <IcePack/ServerManagerF.h>
+// TODO: ML: This file has DOS line endings. Change to Unix line endings.
+
namespace IcePack
{
diff --git a/cpp/src/IcePack/ComponentDeployer.cpp b/cpp/src/IcePack/ComponentDeployer.cpp
index 36e9699a839..d5973d57fde 100644
--- a/cpp/src/IcePack/ComponentDeployer.cpp
+++ b/cpp/src/IcePack/ComponentDeployer.cpp
@@ -69,7 +69,7 @@ public:
if(mkdir(_name.c_str(), 0755) != 0)
{
DeploymentException ex;
- ex.reason = "Couldn't create directory " + _name + ": " + strerror(getSystemErrno());
+ ex.reason = "couldn't create directory " + _name + ": " + strerror(getSystemErrno());
throw ex;
}
}
@@ -178,7 +178,7 @@ public:
if(!configfile)
{
DeploymentException ex;
- ex.reason = "Couldn't create configuration file: " + _file;
+ ex.reason = "couldn't create configuration file: " + _file;
throw ex;
}
@@ -228,7 +228,7 @@ public:
catch(const Ice::LocalException& lex)
{
ostringstream os;
- os << "Couldn't contact the yellow service: " << lex << endl;
+ os << "couldn't contact Yellow: " << lex << endl;
OfferDeploymentException ex;
ex.reason = os.str();
@@ -383,7 +383,7 @@ IcePack::ComponentDeployHandler::getAttributeValue(const AttributeList& attrs, c
if(value == 0)
{
- throw DeploySAXParseException("Missing attribute '" + name + "'", _locator);
+ throw DeploySAXParseException("missing attribute '" + name + "'", _locator);
}
return _deployer.substitute(toString(value));
@@ -518,7 +518,10 @@ IcePack::ComponentDeployer::parse(const string& xmlFile, ComponentDeployHandler&
delete parser;
ostringstream os;
- os << xmlFile << ": UnknownException while parsing file.";
+ // TODO: ML: UnknownException? This it not UnknownException,
+ // that's an unknown exception. UnknownException is an Ice
+ // exception.
+ os << xmlFile << ": UnknownException while parsing file";
ParserDeploymentException ex;
ex.component = _variables["name"];
@@ -533,11 +536,12 @@ IcePack::ComponentDeployer::parse(const string& xmlFile, ComponentDeployHandler&
{
ParserDeploymentException ex;
ex.component = _variables["name"];
- ex.reason = xmlFile + ": Parser returned non null error count";
+ ex.reason = xmlFile + ": parse error";
throw ex;
}
}
+// TODO: ML: Add space, loose last const.
void
IcePack::ComponentDeployer::setDocumentLocator(const Locator*const locator)
{
@@ -648,7 +652,7 @@ IcePack::ComponentDeployer::substitute(const string& v) const
if(end == string::npos)
{
- throw DeploySAXParseException("Malformed variable name in the '" + value + "' value", _locator);
+ throw DeploySAXParseException("malformed variable name in the '" + value + "' value", _locator);
}
@@ -656,7 +660,7 @@ IcePack::ComponentDeployer::substitute(const string& v) const
map<string, string>::const_iterator p = _variables.find(name);
if(p == _variables.end())
{
- throw DeploySAXParseException("Unknown variable name in the '" + value + "' value", _locator);
+ throw DeploySAXParseException("unknown variable name in the '" + value + "' value", _locator);
}
value.replace(beg, end - beg + 1, p->second);
diff --git a/cpp/src/IcePack/ComponentDeployer.h b/cpp/src/IcePack/ComponentDeployer.h
index c9e6ad07b16..1e3b1f06eaa 100644
--- a/cpp/src/IcePack/ComponentDeployer.h
+++ b/cpp/src/IcePack/ComponentDeployer.h
@@ -31,6 +31,9 @@ public:
virtual void undeploy() = 0;
};
+// TODO: ML: Nonportable. The incRef/decRef declarations must be done
+// before the typedef below. Why not just use IceUtil::Handle in this
+// case?
typedef ::IceInternal::Handle< ::IcePack::Task> TaskPtr;
void incRef(::IcePack::Task*);
@@ -42,6 +45,9 @@ class DeploySAXParseException : public SAXParseException
{
public:
+ // TODO: ML: Space is missing: "const Locator* const
+ // locator". Also, why not just const Locator*? What's the point
+ // of making the pointer constant as well?
DeploySAXParseException(const std::string&, const Locator*const locator);
};
@@ -83,10 +89,16 @@ public:
ComponentDeployHandler(ComponentDeployer&);
+ // TODO: ML: Incorrect spacing, should be "const XMLCh*
+ // const". Also, loose the last const -- there is no point in
+ // forcing the pointer itself to be const. (Here and everywhere
+ // else.)
virtual void characters(const XMLCh *const, const unsigned int);
+ // TODO: ML: AttributeList&.
virtual void startElement(const XMLCh *const, AttributeList &);
virtual void endElement(const XMLCh *const);
+ // TODO: ML: No reason to make inline, see style guide.
virtual void ignorableWhitespace(const XMLCh *const, const unsigned int) { }
virtual void processingInstruction(const XMLCh *const, const XMLCh *const) { }
virtual void resetDocument() { }
diff --git a/cpp/src/IcePack/LocatorI.h b/cpp/src/IcePack/LocatorI.h
index 5825c2374b9..206c5344bfb 100644
--- a/cpp/src/IcePack/LocatorI.h
+++ b/cpp/src/IcePack/LocatorI.h
@@ -20,7 +20,7 @@ namespace IcePack
class LocatorI : public ::Ice::Locator
{
-public:
+public: // TODO: ML: Add empty line.
LocatorI(const AdapterManagerPrx&, const ::Ice::LocatorRegistryPrx&);
virtual ::Ice::ObjectPrx findAdapterByName(const std::string&, const ::Ice::Current&);
diff --git a/cpp/src/IcePack/LocatorRegistryI.h b/cpp/src/IcePack/LocatorRegistryI.h
index fa035860de5..bf53fb5d0ef 100644
--- a/cpp/src/IcePack/LocatorRegistryI.h
+++ b/cpp/src/IcePack/LocatorRegistryI.h
@@ -21,7 +21,7 @@ namespace IcePack
class LocatorRegistryI : public Ice::LocatorRegistry
{
-public:
+public: // TODO: ML: Add empty line.
LocatorRegistryI(const AdapterManagerPrx&);
virtual void addAdapter(const ::std::string&, const ::Ice::ObjectPrx&, const ::Ice::Current&);
diff --git a/cpp/src/IcePack/Parser.cpp b/cpp/src/IcePack/Parser.cpp
index 245bb940d0c..c709b0abaaa 100644
--- a/cpp/src/IcePack/Parser.cpp
+++ b/cpp/src/IcePack/Parser.cpp
@@ -41,20 +41,19 @@ void
IcePack::Parser::usage()
{
cout <<
- "help Print this message.\n"
- "exit, quit Exit this program.\n"
- "server add NAME PATH LIBRARY_PATH DESCRIPTOR\n"
- " Add server NAME with PATH.\n"
- "server describe NAME Get server NAME description.\n"
- "server state NAME Get server NAME state.\n"
- "server start NAME Starts server NAME.\n"
- "server remove NAME Remove server NAME.\n"
- "server list List all server names.\n"
- "adapter add NAME ENDPOINTS Add adapter NAME with ENDPOINTS.\n"
- "adapter list List all adapter names.\n"
- "adapter remove NAME Remove adapter NAME.\n"
- "adapter endpoints NAME Get adapter NAME endpoints.\n"
- "shutdown Shut the IcePack server down.\n";
+ "help Print this message.\n"
+ "exit, quit Exit this program.\n"
+ "server add NAME PATH LIBPATH DESC Add server NAME with PATH.\n" // TODO: ML: What about LIBPATH, DESC?
+ "server describe NAME Get server NAME description.\n"
+ "server state NAME Get server NAME state.\n"
+ "server start NAME Starts server NAME.\n"
+ "server remove NAME Remove server NAME.\n"
+ "server list List all server names.\n"
+ "adapter add NAME ENDPOINTS Add adapter NAME with ENDPOINTS.\n"
+ "adapter list List all adapter names.\n"
+ "adapter remove NAME Remove adapter NAME.\n"
+ "adapter endpoints NAME Get adapter NAME endpoints.\n"
+ "shutdown Shut the IcePack server down.\n";
}
void
@@ -63,7 +62,7 @@ IcePack::Parser::addServer(const list<string>& args, const std::list<std::string
{
if(args.size() != 4)
{
- error("`server add' requires four arguments (type `help' for more info)");
+ error("`server add' requires four arguments\n(`help' for more info)");
return;
}
@@ -103,7 +102,7 @@ IcePack::Parser::startServer(const list<string>& args)
{
if(args.size() != 1)
{
- error("`server start' requires exactly one argument (type `help' for more info)");
+ error("`server start' requires exactly one argument\n(`help' for more info)");
return;
}
@@ -111,6 +110,9 @@ IcePack::Parser::startServer(const list<string>& args)
{
if(!_admin->startServer(args.front()))
{
+ // TODO: ML: That's not in our style guide, but don't
+ // capitalize error messages, i.e., use "the server
+ // didn't...". (Here and in other places.)
error("The server didn't start successfully");
}
}
@@ -127,7 +129,7 @@ IcePack::Parser::describeServer(const list<string>& args)
{
if(args.size() != 1)
{
- error("`server describe' requires exactly one argument (type `help' for more info)");
+ error("`server describe' requires exactly one argument\n(`help' for more info)");
return;
}
@@ -160,7 +162,7 @@ IcePack::Parser::stateServer(const list<string>& args)
{
if(args.size() != 1)
{
- error("`server state' requires exactly one argument (type `help' for more info)");
+ error("`server state' requires exactly one argument\n(`help' for more info)");
return;
}
@@ -170,20 +172,21 @@ IcePack::Parser::stateServer(const list<string>& args)
switch(state)
{
+ // TODO: ML: Indentation wrong, { } missing.
case Inactive:
- cout << "Inactive" << endl;
+ cout << "inactive" << endl;
break;
case Activating:
- cout << "Activating" << endl;
+ cout << "activating" << endl;
break;
case Active:
- cout << "Active" << endl;
+ cout << "active" << endl;
break;
case Deactivating:
- cout << "Deactivating" << endl;
+ cout << "deactivating" << endl;
break;
case Destroyed:
- cout << "Destroyed" << endl;
+ cout << "destroyed" << endl;
break;
default:
assert(false);
@@ -202,7 +205,7 @@ IcePack::Parser::removeServer(const list<string>& args)
{
if(args.size() != 1)
{
- error("`server remove' requires exactly one argument (type `help' for more info)");
+ error("`server remove' requires exactly one argument\n(`help' for more info)");
return;
}
@@ -239,7 +242,7 @@ IcePack::Parser::addAdapter(const list<string>& args)
{
if(args.size() < 2)
{
- error("`adapter add' requires at least two arguments (type `help' for more info)");
+ error("`adapter add' requires at least two arguments\n(`help' for more info)");
return;
}
@@ -265,7 +268,7 @@ IcePack::Parser::endpointsAdapter(const list<string>& args)
{
if(args.size() != 1)
{
- error("`adapter endpoints' requires exactly one argument (type `help' for more info)");
+ error("`adapter endpoints' requires exactly one argument\n(`help' for more info)");
return;
}
@@ -287,7 +290,7 @@ IcePack::Parser::removeAdapter(const list<string>& args)
{
if(args.size() != 1)
{
- error("`adapter remove' requires exactly one argument (type `help' for more info)");
+ error("`adapter remove' requires exactly one argument\n(`help' for more info)");
return;
}
diff --git a/cpp/src/IcePack/Server.cpp b/cpp/src/IcePack/Server.cpp
index 16502266c9c..fefa7aada7f 100644
--- a/cpp/src/IcePack/Server.cpp
+++ b/cpp/src/IcePack/Server.cpp
@@ -124,7 +124,7 @@ main(int argc, char* argv[])
}
else
{
- cerr << argv[0] << ": IcePack.Data doesn't contain a valid directory path." << endl;
+ cerr << argv[0] << ": `IcePack.Data' doesn't contain a valid directory path." << endl;
return EXIT_FAILURE;
}
diff --git a/cpp/src/IcePack/ServerDeployer.cpp b/cpp/src/IcePack/ServerDeployer.cpp
index 608d39c697d..de78b8726af 100644
--- a/cpp/src/IcePack/ServerDeployer.cpp
+++ b/cpp/src/IcePack/ServerDeployer.cpp
@@ -44,13 +44,13 @@ public:
{
AdapterDeploymentException ex;
ex.adapter = _desc.name;
- ex.reason = "Adapter already exist";
+ ex.reason = "adapter already exist";
throw ex;
}
catch(const Ice::LocalException& lex)
{
ostringstream os;
- os << "Couldn't contact the adpater manager: " << lex << endl;
+ os << "couldn't contact the adpater manager: " << lex << endl;
AdapterDeploymentException ex;
ex.adapter = _desc.name;
@@ -277,7 +277,7 @@ IcePack::ServerDeployer::setClassName(const string& name)
if(name.empty())
{
- throw DeploySAXParseException("Empty classname element value", _locator);
+ throw DeploySAXParseException("empty classname element value", _locator);
}
_className = name;
@@ -288,7 +288,7 @@ IcePack::ServerDeployer::setWorkingDirectory(const string& pwd)
{
if(pwd.empty())
{
- throw DeploySAXParseException("Empty working directory", _locator);
+ throw DeploySAXParseException("no working directory", _locator);
}
_description.pwd = pwd;
@@ -304,7 +304,7 @@ IcePack::ServerDeployer::addAdapter(const string& name, const string& endpoints)
if(name.empty())
{
- throw DeploySAXParseException("Empty adapter name", _locator);
+ throw DeploySAXParseException("no adapter name", _locator);
}
AdapterDescription desc;
@@ -327,16 +327,17 @@ IcePack::ServerDeployer::addService(const string& name, const string& descriptor
{
if(_kind != ServerKindCppIceBox && _kind != ServerKindJavaIceBox)
{
- throw DeploySAXParseException("Service are only allows in IceBox servers", _locator);
+ // TODO: ML: Grammar. Should this be "allowed"?
+ throw DeploySAXParseException("services are only allows in IceBox servers", _locator);
}
if(name.empty())
{
- throw DeploySAXParseException("Name attribute value is empty", _locator);
+ throw DeploySAXParseException("name attribute value is empty", _locator);
}
if(descriptor.empty())
{
- throw DeploySAXParseException("Descriptor attribute value is empty", _locator);
+ throw DeploySAXParseException("descriptor attribute value is empty", _locator);
}
//
diff --git a/cpp/src/IcePack/ServerManagerI.cpp b/cpp/src/IcePack/ServerManagerI.cpp
index 890d73c582b..6dd9a240582 100644
--- a/cpp/src/IcePack/ServerManagerI.cpp
+++ b/cpp/src/IcePack/ServerManagerI.cpp
@@ -58,9 +58,6 @@ public:
{
}
- //
- // Operations from ObjectFactory
- //
virtual Ice::ObjectPtr
create(const std::string& type)
{
@@ -106,7 +103,7 @@ IcePack::ServerI::start(const Current&)
while(true)
{
IceUtil::Monitor< IceUtil::Mutex>::Lock sync(*this);
- if(!_activator)
+ if(!_activator) // TODO: ML: { } missing, here and in several places below.
return false;
switch(_state)
diff --git a/cpp/src/IcePack/ServerManagerI.h b/cpp/src/IcePack/ServerManagerI.h
index 9ee4105c56d..e4ca7816e09 100644
--- a/cpp/src/IcePack/ServerManagerI.h
+++ b/cpp/src/IcePack/ServerManagerI.h
@@ -23,7 +23,7 @@ namespace IcePack
class ServerI : public Server, public ::IceUtil::Monitor< ::IceUtil::Mutex>
{
-public:
+public: // TODO: ML: Empty line missing.
ServerI(const ::Ice::ObjectAdapterPtr&, const ActivatorPrx&);
virtual ~ServerI();
diff --git a/cpp/src/IcePack/ServiceDeployer.cpp b/cpp/src/IcePack/ServiceDeployer.cpp
index 817799f6b1d..6c39de8f07d 100644
--- a/cpp/src/IcePack/ServiceDeployer.cpp
+++ b/cpp/src/IcePack/ServiceDeployer.cpp
@@ -131,7 +131,7 @@ IcePack::ServiceDeployer::setDBEnv(const string& dir)
{
if(_kind != ServiceKindFreeze)
{
- throw DeploySAXParseException("Database environment is only allowed for Freeze services", _locator);
+ throw DeploySAXParseException("database environment is only allowed for Freeze services", _locator);
}
string path;
diff --git a/cpp/src/IcePack/ServiceDeployer.h b/cpp/src/IcePack/ServiceDeployer.h
index 6895d868585..e72af463eb7 100644
--- a/cpp/src/IcePack/ServiceDeployer.h
+++ b/cpp/src/IcePack/ServiceDeployer.h
@@ -13,6 +13,7 @@
#include <IceUtil/Shared.h>
#include <IcePack/ComponentDeployer.h>
+// Remove duplicate empty line.
namespace IcePack