AddTo will return AddToTypeProxy for convenience Change-Id: If86496af0c68af31a3e0c618b0fae861975a4ebf Reviewed-on: https://weave-review.googlesource.com/2321 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/include/weave/error.h b/include/weave/error.h index 64b99bd..0cf8dc5 100644 --- a/include/weave/error.h +++ b/include/weave/error.h
@@ -24,6 +24,21 @@ public: ~Error() = default; + class AddToTypeProxy { + public: + operator bool() const { return false; } + + template <class T> + operator std::unique_ptr<T>() const { + return nullptr; + } + + template <class T> + operator T*() const { + return nullptr; + } + }; + // Creates an instance of Error class. static ErrorPtr Create(const tracked_objects::Location& location, const std::string& code, @@ -35,17 +50,17 @@ // If |error| is not nullptr, creates another instance of Error class, // initializes it with specified arguments and adds it to the head of // the error chain pointed to by |error|. - static void AddTo(ErrorPtr* error, - const tracked_objects::Location& location, - const std::string& code, - const std::string& message); + static AddToTypeProxy AddTo(ErrorPtr* error, + const tracked_objects::Location& location, + const std::string& code, + const std::string& message); // Same as the Error::AddTo above, but allows to pass in a printf-like // format string and optional parameters to format the error message. - static void AddToPrintf(ErrorPtr* error, - const tracked_objects::Location& location, - const std::string& code, - const char* format, - ...) PRINTF_FORMAT(4, 5); + static AddToTypeProxy AddToPrintf(ErrorPtr* error, + const tracked_objects::Location& location, + const std::string& code, + const char* format, + ...) PRINTF_FORMAT(4, 5); // Clones error with all inner errors. ErrorPtr Clone() const;
diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc index dba14c4..fc9b0e7 100644 --- a/src/commands/command_instance.cc +++ b/src/commands/command_instance.cc
@@ -37,10 +37,10 @@ bool ReportInvalidStateTransition(ErrorPtr* error, Command::State from, Command::State to) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, - "State switch impossible: '%s' -> '%s'", - EnumToString(from).c_str(), EnumToString(to).c_str()); - return false; + return Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, + "State switch impossible: '%s' -> '%s'", + EnumToString(from).c_str(), + EnumToString(to).c_str()); } } // namespace @@ -152,10 +152,9 @@ // Make sure the "parameters" property is actually an object. const base::DictionaryValue* params_dict = nullptr; if (!params_value->GetAsDictionary(¶ms_dict)) { - Error::AddToPrintf(error, FROM_HERE, errors::json::kObjectExpected, - "Property '%s' must be a JSON object", - commands::attributes::kCommand_Parameters); - return params; + return Error::AddToPrintf(error, FROM_HERE, errors::json::kObjectExpected, + "Property '%s' must be a JSON object", + commands::attributes::kCommand_Parameters); } params.reset(params_dict->DeepCopy()); } else { @@ -180,10 +179,9 @@ // Get the command JSON object from the value. const base::DictionaryValue* json = nullptr; if (!value->GetAsDictionary(&json)) { - Error::AddTo(error, FROM_HERE, errors::json::kObjectExpected, - "Command instance is not a JSON object"); command_id->clear(); - return instance; + return Error::AddTo(error, FROM_HERE, errors::json::kObjectExpected, + "Command instance is not a JSON object"); } // Get the command ID from 'id' property. @@ -193,16 +191,15 @@ // Get the command name from 'name' property. std::string command_name; if (!json->GetString(commands::attributes::kCommand_Name, &command_name)) { - Error::AddTo(error, FROM_HERE, errors::commands::kPropertyMissing, - "Command name is missing"); - return instance; + return Error::AddTo(error, FROM_HERE, errors::commands::kPropertyMissing, + "Command name is missing"); } auto parameters = GetCommandParameters(json, error); if (!parameters) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kCommandFailed, - "Failed to validate command '%s'", command_name.c_str()); - return instance; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kCommandFailed, + "Failed to validate command '%s'", command_name.c_str()); } instance.reset(new CommandInstance{command_name, origin, *parameters});
diff --git a/src/component_manager_impl.cc b/src/component_manager_impl.cc index a4f5740..550775d 100644 --- a/src/component_manager_impl.cc +++ b/src/component_manager_impl.cc
@@ -47,18 +47,17 @@ return false; } if (root->GetWithoutPathExpansion(name, nullptr)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, - "Component '%s' already exists at path '%s'", - name.c_str(), path.c_str()); - return false; + return Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, + "Component '%s' already exists at path '%s'", + name.c_str(), path.c_str()); } // Check to make sure the declared traits are already defined. for (const std::string& trait : traits) { if (!FindTraitDefinition(trait)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidPropValue, - "Trait '%s' is undefined", trait.c_str()); - return false; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kInvalidPropValue, + "Trait '%s' is undefined", trait.c_str()); } } std::unique_ptr<base::DictionaryValue> dict{new base::DictionaryValue}; @@ -108,10 +107,9 @@ } if (!root->RemoveWithoutPathExpansion(name, nullptr)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, - "Component '%s' does not exist at path '%s'", - name.c_str(), path.c_str()); - return false; + return Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, + "Component '%s' does not exist at path '%s'", + name.c_str(), path.c_str()); } for (const auto& cb : on_componet_tree_changed_) @@ -132,18 +130,17 @@ base::ListValue* array_value = nullptr; if (!root->GetListWithoutPathExpansion(name, &array_value)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidState, - "There is no component array named '%s' at path '%s'", - name.c_str(), path.c_str()); - return false; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kInvalidState, + "There is no component array named '%s' at path '%s'", name.c_str(), + path.c_str()); } if (!array_value->Remove(index, nullptr)) { - Error::AddToPrintf( + return Error::AddToPrintf( error, FROM_HERE, errors::commands::kInvalidState, "Component array '%s' at path '%s' does not have an element %zu", name.c_str(), path.c_str(), index); - return false; } for (const auto& cb : on_componet_tree_changed_) @@ -233,11 +230,10 @@ return nullptr; if (role < minimal_role) { - Error::AddToPrintf(error, FROM_HERE, "access_denied", - "User role '%s' less than minimal: '%s'", - EnumToString(role).c_str(), - EnumToString(minimal_role).c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, "access_denied", + "User role '%s' less than minimal: '%s'", + EnumToString(role).c_str(), + EnumToString(minimal_role).c_str()); } std::string component_path = command_instance->GetComponent(); @@ -248,12 +244,11 @@ SplitAtFirst(command_instance->GetName(), ".", true).first; component_path = FindComponentWithTrait(trait_name); if (component_path.empty()) { - Error::AddToPrintf( + return Error::AddToPrintf( error, FROM_HERE, "unrouted_command", "Unable route command '%s' because there is no component supporting" "trait '%s'", command_instance->GetName().c_str(), trait_name.c_str()); - return nullptr; } command_instance->SetComponent(component_path); } @@ -279,10 +274,9 @@ } if (!trait_supported) { - Error::AddToPrintf(error, FROM_HERE, "trait_not_supported", - "Component '%s' doesn't support trait '%s'", - component_path.c_str(), pair.first.c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, "trait_not_supported", + "Component '%s' doesn't support trait '%s'", + component_path.c_str(), pair.first.c_str()); } if (command_id.empty()) { @@ -353,10 +347,9 @@ ErrorPtr* error) const { const base::DictionaryValue* command = FindCommandDefinition(command_name); if (!command) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kInvalidCommandName, - "Command definition for '%s' not found", - command_name.c_str()); - return false; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kInvalidCommandName, + "Command definition for '%s' not found", command_name.c_str()); } std::string value; // The JSON definition has been pre-validated already in LoadCommands, so @@ -414,22 +407,22 @@ return nullptr; auto pair = SplitAtFirst(name, ".", true); if (pair.first.empty()) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Empty state package in '%s'", name.c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kPropertyMissing, + "Empty state package in '%s'", name.c_str()); } if (pair.second.empty()) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "State property name not specified in '%s'", - name.c_str()); - return nullptr; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kPropertyMissing, + "State property name not specified in '%s'", name.c_str()); } std::string key = base::StringPrintf("state.%s", name.c_str()); const base::Value* value = nullptr; if (!component->Get(key, &value)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "State property '%s' not found in component '%s'", - name.c_str(), component_path.c_str()); + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kPropertyMissing, + "State property '%s' not found in component '%s'", + name.c_str(), component_path.c_str()); } return value; } @@ -441,15 +434,14 @@ base::DictionaryValue dict; auto pair = SplitAtFirst(name, ".", true); if (pair.first.empty()) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Empty state package in '%s'", name.c_str()); - return false; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kPropertyMissing, + "Empty state package in '%s'", name.c_str()); } if (pair.second.empty()) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "State property name not specified in '%s'", - name.c_str()); - return false; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kPropertyMissing, + "State property name not specified in '%s'", name.c_str()); } dict.Set(name, value.DeepCopy()); return SetStateProperties(component_path, dict, error); @@ -673,26 +665,24 @@ auto element = SplitAtFirst(parts[i], "[", true); int array_index = -1; if (element.first.empty()) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Empty path element at '%s'", root_path.c_str()); - return nullptr; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kPropertyMissing, + "Empty path element at '%s'", root_path.c_str()); } if (!element.second.empty()) { if (element.second.back() != ']') { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Invalid array element syntax '%s'", - parts[i].c_str()); - return nullptr; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kPropertyMissing, + "Invalid array element syntax '%s'", parts[i].c_str()); } element.second.pop_back(); std::string index_str; base::TrimWhitespaceASCII(element.second, base::TrimPositions::TRIM_ALL, &index_str); if (!base::StringToInt(index_str, &array_index) || array_index < 0) { - Error::AddToPrintf(error, FROM_HERE, - errors::commands::kInvalidPropValue, - "Invalid array index '%s'", element.second.c_str()); - return nullptr; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kInvalidPropValue, + "Invalid array index '%s'", element.second.c_str()); } } @@ -701,10 +691,10 @@ // points to the actual parent component. We need the root to point to // the 'components' element containing child sub-components instead. if (!root->GetDictionary("components", &root)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Component '%s' does not exist at '%s'", - element.first.c_str(), root_path.c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kPropertyMissing, + "Component '%s' does not exist at '%s'", + element.first.c_str(), root_path.c_str()); } } @@ -717,16 +707,16 @@ } if (value->GetType() == base::Value::TYPE_LIST && array_index < 0) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kTypeMismatch, - "Element '%s.%s' is an array", root_path.c_str(), - element.first.c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kTypeMismatch, + "Element '%s.%s' is an array", + root_path.c_str(), element.first.c_str()); } if (value->GetType() == base::Value::TYPE_DICTIONARY && array_index >= 0) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kTypeMismatch, - "Element '%s.%s' is not an array", root_path.c_str(), - element.first.c_str()); - return nullptr; + return Error::AddToPrintf(error, FROM_HERE, + errors::commands::kTypeMismatch, + "Element '%s.%s' is not an array", + root_path.c_str(), element.first.c_str()); } if (value->GetType() == base::Value::TYPE_DICTIONARY) { @@ -737,11 +727,10 @@ const base::Value* component_value = nullptr; if (!component_array->Get(array_index, &component_value) || !component_value->GetAsDictionary(&root)) { - Error::AddToPrintf(error, FROM_HERE, errors::commands::kPropertyMissing, - "Element '%s.%s' does not contain item #%d", - root_path.c_str(), element.first.c_str(), - array_index); - return nullptr; + return Error::AddToPrintf( + error, FROM_HERE, errors::commands::kPropertyMissing, + "Element '%s.%s' does not contain item #%d", root_path.c_str(), + element.first.c_str(), array_index); } } if (!root_path.empty())
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 0c4a5d7..7c20084 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc
@@ -196,10 +196,9 @@ SplitAtFirst(response.GetContentType(), ";", true).first; if (content_type != http::kJson && content_type != http::kPlain) { - Error::AddTo( + return Error::AddTo( error, FROM_HERE, "non_json_content_type", "Unexpected content type: \'" + response.GetContentType() + "\'"); - return std::unique_ptr<base::DictionaryValue>(); } const std::string& json = response.GetData(); @@ -328,10 +327,11 @@ VLOG(2) << "Device registration record " << ((have_credentials) ? "found" : "not found."); - if (!have_credentials) - Error::AddTo(error, FROM_HERE, "device_not_registered", - "No valid device registration record found"); - return have_credentials; + if (!have_credentials) { + return Error::AddTo(error, FROM_HERE, "device_not_registered", + "No valid device registration record found"); + } + return true; } std::unique_ptr<base::DictionaryValue> @@ -352,8 +352,7 @@ if (!resp->GetString("error_description", &error_message)) { error_message = "Unexpected OAuth error"; } - Error::AddTo(error, FROM_HERE, error_code, error_message); - return std::unique_ptr<base::DictionaryValue>(); + return Error::AddTo(error, FROM_HERE, error_code, error_message); } return resp; } @@ -836,9 +835,8 @@ const std::string& service_url, ErrorPtr* error) { if (HaveRegistrationCredentials()) { - Error::AddTo(error, FROM_HERE, kErrorAlreayRegistered, - "Unable to change config for registered device"); - return false; + return Error::AddTo(error, FROM_HERE, kErrorAlreayRegistered, + "Unable to change config for registered device"); } Config::Transaction change{config_}; change.set_client_id(client_id);
diff --git a/src/error.cc b/src/error.cc index ced91e5..cb279c1 100644 --- a/src/error.cc +++ b/src/error.cc
@@ -39,10 +39,10 @@ return ErrorPtr(new Error(location, code, message, std::move(inner_error))); } -void Error::AddTo(ErrorPtr* error, - const tracked_objects::Location& location, - const std::string& code, - const std::string& message) { +Error::AddToTypeProxy Error::AddTo(ErrorPtr* error, + const tracked_objects::Location& location, + const std::string& code, + const std::string& message) { if (error) { *error = Create(location, code, message, std::move(*error)); } else { @@ -50,18 +50,21 @@ // we still want to log the error... LogError(location, code, message); } + return {}; } -void Error::AddToPrintf(ErrorPtr* error, - const tracked_objects::Location& location, - const std::string& code, - const char* format, - ...) { +Error::AddToTypeProxy Error::AddToPrintf( + ErrorPtr* error, + const tracked_objects::Location& location, + const std::string& code, + const char* format, + ...) { va_list ap; va_start(ap, format); std::string message = base::StringPrintV(format, ap); va_end(ap); AddTo(error, location, code, message); + return {}; } ErrorPtr Error::Clone() const {
diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc index 0753a2b..66d04c4 100644 --- a/src/privet/auth_manager.cc +++ b/src/privet/auth_manager.cc
@@ -65,14 +65,13 @@ ErrorPtr* error) { UwMacaroonCaveatType caveat_type{}; if (!uw_macaroon_caveat_get_type_(&caveat, &caveat_type)) { - Error::AddTo(error, FROM_HERE, kInvalidTokenError, "Unable to get type"); - return false; + return Error::AddTo(error, FROM_HERE, kInvalidTokenError, + "Unable to get type"); } if (caveat_type != type) { - Error::AddTo(error, FROM_HERE, kInvalidTokenError, - "Unexpected caveat type"); - return false; + return Error::AddTo(error, FROM_HERE, kInvalidTokenError, + "Unexpected caveat type"); } return true; @@ -86,8 +85,8 @@ return false; if (!uw_macaroon_caveat_get_value_uint_(&caveat, value)) { - Error::AddTo(error, FROM_HERE, kInvalidTokenError, "Unable to read caveat"); - return false; + return Error::AddTo(error, FROM_HERE, kInvalidTokenError, + "Unable to read caveat"); } return true; @@ -103,8 +102,8 @@ const uint8_t* start{nullptr}; size_t size{0}; if (!uw_macaroon_caveat_get_value_str_(&caveat, &start, &size)) { - Error::AddTo(error, FROM_HERE, kInvalidTokenError, "Unable to read caveat"); - return false; + return Error::AddTo(error, FROM_HERE, kInvalidTokenError, + "Unable to read caveat"); } value->assign(reinterpret_cast<const char*>(start), size); @@ -144,8 +143,8 @@ buffer->resize(kMaxMacaroonSize); if (!uw_macaroon_load_(token.data(), token.size(), buffer->data(), buffer->size(), macaroon)) { - Error::AddTo(error, FROM_HERE, kInvalidTokenError, "Invalid token format"); - return false; + return Error::AddTo(error, FROM_HERE, kInvalidTokenError, + "Invalid token format"); } return true; } @@ -155,9 +154,8 @@ ErrorPtr* error) { CHECK_EQ(kSha256OutputSize, secret.size()); if (!uw_macaroon_verify_(&macaroon, secret.data(), secret.size())) { - Error::AddTo(error, FROM_HERE, "invalid_signature", - "Invalid token signature"); - return false; + return Error::AddTo(error, FROM_HERE, "invalid_signature", + "Invalid token signature"); } return true; } @@ -271,23 +269,20 @@ &user_id, error) || !ReadCaveat(macaroon.caveats[2], kUwMacaroonCaveatTypeExpiration, &expiration, error)) { - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthorization, - "Invalid token"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthorization, + "Invalid token"); } AuthScope auth_scope{FromMacaroonScope(scope)}; if (auth_scope == AuthScope::kNone) { - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthorization, - "Invalid token data"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthorization, + "Invalid token data"); } base::Time time{base::Time::FromTimeT(expiration)}; if (time < clock_->Now()) { - Error::AddTo(error, FROM_HERE, errors::kAuthorizationExpired, - "Token is expired"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kAuthorizationExpired, + "Token is expired"); } if (user_info) @@ -329,8 +324,7 @@ return auth.first->IsValidAuthToken(token, nullptr); }); if (claim == pending_claims_.end()) { - Error::AddTo(error, FROM_HERE, errors::kNotFound, "Unknown claim"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kNotFound, "Unknown claim"); } SetAuthSecret(claim->first->GetAuthSecret(), claim->second); @@ -358,8 +352,8 @@ UwMacaroon macaroon{}; if (!LoadMacaroon(token, &buffer, &macaroon, error) || !VerifyMacaroon(auth_secret_, macaroon, error)) { - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthCode, "Invalid token"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthCode, + "Invalid token"); } return true; }
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc index 0eb04e0..5f31fee 100644 --- a/src/privet/cloud_delegate.cc +++ b/src/privet/cloud_delegate.cc
@@ -320,9 +320,8 @@ return true; } - Error::AddTo(error, FROM_HERE, errors::kAccessDenied, - "Need to be owner of the command."); - return false; + return Error::AddTo(error, FROM_HERE, errors::kAccessDenied, + "Need to be owner of the command."); } provider::TaskRunner* task_runner_{nullptr};
diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index a353a0f..fa79e77 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc
@@ -197,11 +197,9 @@ TEST_F(PrivetHandlerTest, ExpiredAuth) { auth_header_ = "Privet 123"; EXPECT_CALL(security_, ParseAccessToken(_, _, _)) - .WillRepeatedly(DoAll(WithArgs<2>(Invoke([](ErrorPtr* error) { - Error::AddTo(error, FROM_HERE, - "authorizationExpired", ""); - })), - Return(false))); + .WillRepeatedly(WithArgs<2>(Invoke([](ErrorPtr* error) { + return Error::AddTo(error, FROM_HERE, "authorizationExpired", ""); + }))); EXPECT_PRED2(IsEqualError, CodeWithReason(403, "authorizationExpired"), HandleRequest("/privet/info", "{}")); } @@ -379,10 +377,10 @@ TEST_F(PrivetHandlerTest, AuthErrorInvalidAuthCode) { auto set_error = [](ErrorPtr* error) { - Error::AddTo(error, FROM_HERE, "invalidAuthCode", ""); + return Error::AddTo(error, FROM_HERE, "invalidAuthCode", ""); }; EXPECT_CALL(security_, CreateAccessToken(_, "testToken", _, _, _, _, _)) - .WillRepeatedly(DoAll(WithArgs<6>(Invoke(set_error)), Return(false))); + .WillRepeatedly(WithArgs<6>(Invoke(set_error))); const char kInput[] = R"({ 'mode': 'pairing', 'requestedScope': 'user', @@ -601,10 +599,9 @@ } })"; auto set_error = [](const std::string&, const std::string&, ErrorPtr* error) { - Error::AddTo(error, FROM_HERE, "deviceBusy", ""); + return Error::AddTo(error, FROM_HERE, "deviceBusy", ""); }; - EXPECT_CALL(wifi_, ConfigureCredentials(_, _, _)) - .WillOnce(DoAll(Invoke(set_error), Return(false))); + EXPECT_CALL(wifi_, ConfigureCredentials(_, _, _)).WillOnce(Invoke(set_error)); EXPECT_PRED2(IsEqualError, CodeWithReason(503, "deviceBusy"), HandleRequest("/privet/v3/setup/start", kInput)); @@ -641,10 +638,9 @@ })"; auto set_error = [](const std::string&, const std::string&, ErrorPtr* error) { - Error::AddTo(error, FROM_HERE, "deviceBusy", ""); + return Error::AddTo(error, FROM_HERE, "deviceBusy", ""); }; - EXPECT_CALL(cloud_, Setup(_, _, _)) - .WillOnce(DoAll(Invoke(set_error), Return(false))); + EXPECT_CALL(cloud_, Setup(_, _, _)).WillOnce(Invoke(set_error)); EXPECT_PRED2(IsEqualError, CodeWithReason(503, "deviceBusy"), HandleRequest("/privet/v3/setup/start", kInput)); @@ -857,8 +853,7 @@ "{'path':'comp1.comp2', 'filter':['traits', 'components']}")); auto error_handler = [](ErrorPtr* error) -> const base::DictionaryValue* { - Error::AddTo(error, FROM_HERE, "componentNotFound", ""); - return nullptr; + return Error::AddTo(error, FROM_HERE, "componentNotFound", ""); }; EXPECT_CALL(cloud_, FindComponent("comp7", _)) .WillOnce(WithArgs<1>(Invoke(error_handler)));
diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index 7a3533a..358876d 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc
@@ -51,9 +51,8 @@ case crypto::P224EncryptedKeyExchange::kResultPending: return true; case crypto::P224EncryptedKeyExchange::kResultFailed: - Error::AddTo(error, FROM_HERE, errors::kInvalidClientCommitment, - spake_.error()); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidClientCommitment, + spake_.error()); default: LOG(FATAL) << "SecurityManager uses only one round trip"; } @@ -139,9 +138,8 @@ base::TimeDelta* access_token_ttl, ErrorPtr* error) { auto disabled_mode = [](ErrorPtr* error) { - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthMode, - "Mode is not available"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthMode, + "Mode is not available"); }; switch (auth_type) { @@ -154,9 +152,8 @@ if (!IsPairingAuthSupported()) return disabled_mode(error); if (!IsValidPairingCode(auth_code)) { - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthCode, - "Invalid authCode"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthCode, + "Invalid authCode"); } return CreateAccessTokenImpl(auth_type, desired_scope, access_token, access_token_scope, access_token_ttl); @@ -170,9 +167,8 @@ error); } - Error::AddTo(error, FROM_HERE, errors::kInvalidAuthMode, - "Unsupported auth mode"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthMode, + "Unsupported auth mode"); } bool SecurityManager::CreateAccessToken(AuthType auth_type, @@ -290,9 +286,8 @@ const auto& pairing_modes = GetSettings().pairing_modes; if (std::find(pairing_modes.begin(), pairing_modes.end(), mode) == pairing_modes.end()) { - Error::AddTo(error, FROM_HERE, errors::kInvalidParams, - "Pairing mode is not enabled"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidParams, + "Pairing mode is not enabled"); } std::string code; @@ -305,9 +300,8 @@ code = base::StringPrintf("%04i", base::RandInt(0, 9999)); break; default: - Error::AddTo(error, FROM_HERE, errors::kInvalidParams, - "Unsupported pairing mode"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidParams, + "Unsupported pairing mode"); } std::unique_ptr<KeyExchanger> spake; @@ -322,9 +316,8 @@ } // Fall through... default: - Error::AddTo(error, FROM_HERE, errors::kInvalidParams, - "Unsupported crypto"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kInvalidParams, + "Unsupported crypto"); } // Allow only a single session at a time for now. @@ -382,9 +375,8 @@ if (!session->second->ProcessMessage( std::string(commitment.begin(), commitment.end()), error)) { ClosePendingSession(session_id); - Error::AddTo(error, FROM_HERE, errors::kCommitmentMismatch, - "Pairing code or crypto implementation mismatch"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kCommitmentMismatch, + "Pairing code or crypto implementation mismatch"); } const std::string& key = session->second->GetKey(); @@ -440,9 +432,8 @@ return true; if (block_pairing_until_ > auth_manager_->Now()) { - Error::AddTo(error, FROM_HERE, errors::kDeviceBusy, - "Too many pairing attempts"); - return false; + return Error::AddTo(error, FROM_HERE, errors::kDeviceBusy, + "Too many pairing attempts"); } if (++pairing_attemts_ >= kMaxAllowedPairingAttemts) {
diff --git a/third_party/chromium/base/compiler_specific.h b/third_party/chromium/base/compiler_specific.h index 339e9b7..79b7fa8 100644 --- a/third_party/chromium/base/compiler_specific.h +++ b/third_party/chromium/base/compiler_specific.h
@@ -135,7 +135,7 @@ // |dots_param| is the one-based index of the "..." parameter. // For v*printf functions (which take a va_list), pass 0 for dots_param. // (This is undocumented but matches what the system C headers do.) -#if defined(COMPILER_GCC) +#if defined(COMPILER_GCC) || defined(__clang__) #define PRINTF_FORMAT(format_param, dots_param) \ __attribute__((format(printf, format_param, dots_param))) #else