shill; Create Error::LogMessage utility routine
This combines a LOG(ERROR) with an error->Populate(). It has
the downside of having the error originate from error.cc, as
opposed to the orignal call site of the error, so readers of
the log need to rely on the content of the error message to
tell where the error came from.
BUG=chromium-os:22426
TEST=Rerun unit tests, look at all error messages
Change-Id: I9ccd5385db7c4eda34eb242abd02c57c899d161c
Reviewed-on: https://gerrit.chromium.org/gerrit/11098
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index a6a3c61..666a8ce 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -466,10 +466,8 @@
void Cellular::RegisterOnNetwork(const string &network_id, Error *error) {
LOG(INFO) << __func__ << "(" << network_id << ")";
if (type_ != kTypeGSM) {
- const string kMessage = "Network registration supported only for GSM.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Network registration supported only for GSM.");
return;
}
// Defer registration because we may be in a dbus-c++ callback.
@@ -490,10 +488,8 @@
void Cellular::RequirePIN(const string &pin, bool require, Error *error) {
VLOG(2) << __func__ << "(" << pin << ", " << require << ")";
if (type_ != kTypeGSM) {
- const string kMessage = "RequirePIN supported only for GSM.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "RequirePIN supported only for GSM.");
return;
}
// Defer registration because we may be in a dbus-c++ callback.
@@ -511,10 +507,8 @@
void Cellular::EnterPIN(const string &pin, Error *error) {
VLOG(2) << __func__ << "(" << pin << ")";
if (type_ != kTypeGSM) {
- const string kMessage = "EnterPIN supported only for GSM.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "EnterPIN supported only for GSM.");
return;
}
// Defer registration because we may be in a dbus-c++ callback.
@@ -534,10 +528,8 @@
Error *error) {
VLOG(2) << __func__ << "(" << unblock_code << ", " << pin << ")";
if (type_ != kTypeGSM) {
- const string kMessage = "UnblockPIN supported only for GSM.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "UnblockPIN supported only for GSM.");
return;
}
// Defer registration because we may be in a dbus-c++ callback.
@@ -558,10 +550,8 @@
Error *error) {
VLOG(2) << __func__ << "(" << old_pin << ", " << new_pin << ")";
if (type_ != kTypeGSM) {
- const string kMessage = "ChangePIN supported only for GSM.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "ChangePIN supported only for GSM.");
return;
}
// Defer registration because we may be in a dbus-c++ callback.
@@ -725,18 +715,17 @@
VLOG(2) << __func__;
if (state_ == kStateConnected ||
state_ == kStateLinked) {
- LOG(ERROR) << "Already connected; connection request ignored.";
- CHECK(error);
- error->Populate(Error::kAlreadyConnected);
+ Error::PopulateAndLog(error, Error::kAlreadyConnected,
+ "Already connected; connection request ignored.");
return;
}
CHECK_EQ(kStateRegistered, state_);
if (!allow_roaming_ &&
service_->roaming_state() == flimflam::kRoamingStateRoaming) {
- LOG(ERROR) << "Roaming disallowed; connection request ignored.";
+ Error::PopulateAndLog(error, Error::kNotOnHomeNetwork,
+ "Roaming disallowed; connection request ignored.");
CHECK(error);
- error->Populate(Error::kNotOnHomeNetwork);
return;
}
@@ -804,10 +793,8 @@
void Cellular::Scan(Error *error) {
VLOG(2) << __func__;
if (type_ != kTypeGSM) {
- const string kMessage = "Network scanning support for GSM only.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Network scanning support for GSM only.");
return;
}
// Defer scan because we may be in a dbus-c++ callback.
@@ -892,18 +879,14 @@
void Cellular::Activate(const string &carrier, Error *error) {
VLOG(2) << __func__ << "(" << carrier << ")";
if (type_ != kTypeCDMA) {
- const string kMessage = "Unable to activate non-CDMA modem.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Unable to activate non-CDMA modem.");
return;
}
if (state_ != kStateEnabled &&
state_ != kStateRegistered) {
- const string kMessage = "Unable to activate in " + GetStateString(state_);
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Unable to activate in " + GetStateString(state_));
return;
}
// Defer connect because we may be in a dbus-c++ callback.
diff --git a/device.cc b/device.cc
index cbc98ad..90f6eb1 100644
--- a/device.cc
+++ b/device.cc
@@ -148,50 +148,38 @@
void Device::Scan(Error *error) {
VLOG(2) << "Device " << link_name_ << " scan requested.";
- const string kMessage = "Device doesn't support scan.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support scan.");
}
void Device::RegisterOnNetwork(const std::string &/*network_id*/,
Error *error) {
- const string kMessage = "Device doesn't support network registration.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support network registration.");
}
void Device::RequirePIN(const string &/*pin*/, bool /*require*/, Error *error) {
- const string kMessage = "Device doesn't support RequirePIN.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support RequirePIN.");
}
void Device::EnterPIN(const string &/*pin*/, Error *error) {
- const string kMessage = "Device doesn't support EnterPIN.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support EnterPIN.");
}
void Device::UnblockPIN(const string &/*unblock_code*/,
const string &/*pin*/,
Error *error) {
- const string kMessage = "Device doesn't support UnblockPIN.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support UnblockPIN.");
}
void Device::ChangePIN(const string &/*old_pin*/,
const string &/*new_pin*/,
Error *error) {
- const string kMessage = "Device doesn't support ChangePIN.";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ "Device doesn't support ChangePIN.");
}
string Device::GetRpcIdentifier() {
diff --git a/error.cc b/error.cc
index e486bb5..5ab022e 100644
--- a/error.cc
+++ b/error.cc
@@ -86,4 +86,12 @@
return kInfos[type].message;
}
+// static
+void Error::PopulateAndLog(Error *error, Type type, const string &message) {
+ LOG(ERROR) << message;
+ if (error) {
+ error->Populate(type, message);
+ }
+}
+
} // namespace shill
diff --git a/error.h b/error.h
index c1fb2b1..a8159a0 100644
--- a/error.h
+++ b/error.h
@@ -62,6 +62,10 @@
static std::string GetName(Type type);
static std::string GetDefaultMessage(Type type);
+ // Log an error message. If |error| is non-NULL, also populate it.
+ static void PopulateAndLog(Error *error, Type type,
+ const std::string &message);
+
private:
struct Info {
const char *name; // Error type name.
diff --git a/manager.cc b/manager.cc
index c3cbc12..bd73bd7 100644
--- a/manager.cc
+++ b/manager.cc
@@ -151,10 +151,8 @@
void Manager::CreateProfile(const std::string &name, Error *error) {
Profile::Identifier ident;
if (!Profile::ParseIdentifier(name, &ident)) {
- const string kMessage = "Invalid profile name " + name;
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Invalid profile name " + name);
return;
}
ProfileRefPtr profile(new Profile(control_interface_,
@@ -168,22 +166,17 @@
// Save profile data out, and then let the scoped pointer fall out of scope.
if (!profile->Save()) {
- const string kMessage = "Profile name " + name + " could not be saved";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInternalError, kMessage);
+ Error::PopulateAndLog(error, Error::kInternalError,
+ "Profile name " + name + " could not be saved");
return;
}
}
void Manager::PushProfile(const string &name, Error *error) {
-
Profile::Identifier ident;
if (!Profile::ParseIdentifier(name, &ident)) {
- const string kMessage = "Invalid profile name " + name;
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Invalid profile name " + name);
return;
}
@@ -191,10 +184,8 @@
it != profiles_.end();
++it) {
if ((*it)->MatchesIdentifier(ident)) {
- const string kMessage = "Profile name " + name + " is already on stack";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kAlreadyExists, kMessage);
+ Error::PopulateAndLog(error, Error::kAlreadyExists,
+ "Profile name " + name + " is already on stack");
return;
}
}
@@ -208,10 +199,8 @@
// to avoid leaving permanent side effects to devices under test. This
// whole thing will need to be reworked in order to allow that to happen,
// or the autotests (or their expectations) will need to change.
- const string kMessage = "Cannot load non-default global profile " + name;
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Cannot load non-default global profile " + name);
return;
}
@@ -259,25 +248,18 @@
void Manager::PopProfile(const std::string &name, Error *error) {
Profile::Identifier ident;
if (profiles_.empty()) {
- const string kMessage = "Profile stack is empty";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotFound, kMessage);
+ Error::PopulateAndLog(error, Error::kNotFound, "Profile stack is empty");
return;
}
ProfileRefPtr active_profile = profiles_.back();
if (!Profile::ParseIdentifier(name, &ident)) {
- const string kMessage = "Invalid profile name " + name;
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Invalid profile name " + name);
return;
}
if (!active_profile->MatchesIdentifier(ident)) {
- const string kMessage = name + " is not the active profile";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotSupported, kMessage);
+ Error::PopulateAndLog(error, Error::kNotSupported,
+ name + " is not the active profile");
return;
}
PopProfileInternal();
@@ -286,10 +268,7 @@
void Manager::PopAnyProfile(Error *error) {
Profile::Identifier ident;
if (profiles_.empty()) {
- const string kMessage = "Profile stack is empty";
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kNotFound, kMessage);
+ Error::PopulateAndLog(error, Error::kNotFound, "Profile stack is empty");
return;
}
PopProfileInternal();
@@ -506,10 +485,8 @@
}
} else {
// TODO(quiche): support scanning for other technologies?
- const string kMessage = "Unrecognized technology " + technology;
- LOG(ERROR) << kMessage;
- CHECK(error);
- error->Populate(Error::kInvalidArguments, kMessage);
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ "Unrecognized technology " + technology);
}
}
@@ -537,14 +514,14 @@
Technology::Identifier identifier = Technology::IdentifierFromName(*it);
if (identifier == Technology::kUnknown) {
- error->Populate(Error::kInvalidArguments, *it +
- " is an unknown technology name");
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ *it + " is an unknown technology name");
return;
}
if (ContainsKey(seen, identifier)) {
- error->Populate(Error::kInvalidArguments, *it +
- " is duplicated in the list");
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
+ *it + " is duplicated in the list");
return;
}
seen[identifier] = true;
diff --git a/profile.cc b/profile.cc
index d87bbf2..7ba3aef 100644
--- a/profile.cc
+++ b/profile.cc
@@ -61,13 +61,9 @@
Error *error) {
FilePath final_path;
if (!GetStoragePath(&final_path)) {
- const string kMessage =
+ Error::PopulateAndLog(error, Error::kInvalidArguments,
base::StringPrintf("Could not set up profile storage for %s:%s",
- name_.user.c_str(), name_.identifier.c_str());
- LOG(ERROR) << kMessage;
- if (error) {
- error->Populate(Error::kInvalidArguments, kMessage);
- }
+ name_.user.c_str(), name_.identifier.c_str()));
return false;
}
scoped_ptr<KeyFileStore> storage(new KeyFileStore(glib));
@@ -75,33 +71,21 @@
bool already_exists = storage->IsNonEmpty();
if (!already_exists && storage_option != kCreateNew &&
storage_option != kCreateOrOpenExisting) {
- const string kMessage =
+ Error::PopulateAndLog(error, Error::kNotFound,
base::StringPrintf("Profile storage for %s:%s does not already exist",
- name_.user.c_str(), name_.identifier.c_str());
- LOG(ERROR) << kMessage;
- if (error) {
- error->Populate(Error::kNotFound, kMessage);
- }
+ name_.user.c_str(), name_.identifier.c_str()));
return false;
} else if (already_exists && storage_option != kOpenExisting &&
storage_option != kCreateOrOpenExisting) {
- const string kMessage =
+ Error::PopulateAndLog(error, Error::kAlreadyExists,
base::StringPrintf("Profile storage for %s:%s already exists",
- name_.user.c_str(), name_.identifier.c_str());
- LOG(ERROR) << kMessage;
- if (error) {
- error->Populate(Error::kAlreadyExists, kMessage);
- }
+ name_.user.c_str(), name_.identifier.c_str()));
return false;
}
if (!storage->Open()) {
- const string kMessage =
+ Error::PopulateAndLog(error, Error::kInternalError,
base::StringPrintf("Could not open profile storage for %s:%s",
- name_.user.c_str(), name_.identifier.c_str());
- LOG(ERROR) << kMessage;
- if (error) {
- error->Populate(Error::kInternalError, kMessage);
- }
+ name_.user.c_str(), name_.identifier.c_str()));
return false;
}
if (!already_exists) {