buffet: StorageInterface::Save accepts const reference
Out codebase mostly written with assumption that const parameters
passed as const reference.
Looking at storage_->Save(&dict) reader may believe that dict is the
destination for Save.
BUG=none
TEST=gizmo-emerge buffet
Change-Id: I15b2b1a48e3053979ecd28a8e8775997e7b7e06d
Reviewed-on: https://chromium-review.googlesource.com/270793
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 0ef58d7..80e7e9b 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -236,7 +236,7 @@
dict.SetString(storage_keys::kAnonymousAccessRole,
config_->anonymous_access_role());
- return storage_->Save(&dict);
+ return storage_->Save(dict);
}
void DeviceRegistrationInfo::ScheduleStartDevice(const base::TimeDelta& later) {
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index ec68b21..417aff4 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -186,7 +186,7 @@
protected:
void SetUp() override {
storage_ = std::make_shared<MemStorage>();
- storage_->Save(&data_);
+ storage_->Save(data_);
transport_ = std::make_shared<chromeos::http::fake::Transport>();
command_manager_ = std::make_shared<CommandManager>();
state_manager_ = std::make_shared<StateManager>(&mock_state_change_queue_);
@@ -250,7 +250,7 @@
data.SetString(storage_keys::kLocation, "m");
data.SetString(storage_keys::kAnonymousAccessRole, "user");
- storage_->Save(&data);
+ storage_->Save(data);
// This test isn't really trying to test Load, it is just the easiest
// way to initialize the properties in dev_reg_.
@@ -258,7 +258,7 @@
// Clear the storage to get a clean test.
base::DictionaryValue empty;
- storage_->Save(&empty);
+ storage_->Save(empty);
EXPECT_TRUE(DeviceRegistrationInfo::TestHelper::Save(dev_reg_.get()));
EXPECT_TRUE(storage_->Load()->Equals(&data));
}
@@ -287,7 +287,7 @@
EXPECT_EQ(0, transport_->GetRequestCount());
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -301,7 +301,7 @@
TEST_F(DeviceRegistrationInfoTest, CheckAuthenticationFailure) {
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
@@ -319,7 +319,7 @@
TEST_F(DeviceRegistrationInfoTest, CheckDeregistration) {
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
@@ -338,7 +338,7 @@
TEST_F(DeviceRegistrationInfoTest, GetDeviceInfo) {
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -360,7 +360,7 @@
TEST_F(DeviceRegistrationInfoTest, GetDeviceId) {
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
@@ -513,7 +513,7 @@
dev_reg_->GetRegistrationStatus());
// Put some credentials into our state, make sure we call that offline.
SetDefaultDeviceRegistration(&data_);
- storage_->Save(&data_);
+ storage_->Save(data_);
EXPECT_TRUE(dev_reg_->Load());
EXPECT_EQ(RegistrationStatus::kConnecting, dev_reg_->GetRegistrationStatus());
}
diff --git a/buffet/storage_impls.cc b/buffet/storage_impls.cc
index d95538a..e45ddbf 100644
--- a/buffet/storage_impls.cc
+++ b/buffet/storage_impls.cc
@@ -23,10 +23,10 @@
return std::unique_ptr<base::Value>(base::JSONReader::Read(json));
}
-bool FileStorage::Save(const base::Value* config) {
+bool FileStorage::Save(const base::Value& config) {
std::string json;
base::JSONWriter::WriteWithOptions(
- config, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json);
+ &config, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json);
return base::ImportantFileWriter::WriteFileAtomically(file_path_, json);
}
@@ -35,8 +35,8 @@
return std::unique_ptr<base::Value>(cache_->DeepCopy());
}
-bool MemStorage::Save(const base::Value* config) {
- cache_.reset(config->DeepCopy());
+bool MemStorage::Save(const base::Value& config) {
+ cache_.reset(config.DeepCopy());
++save_count_;
return true;
}
diff --git a/buffet/storage_impls.h b/buffet/storage_impls.h
index 8b90a6c..f4034be 100644
--- a/buffet/storage_impls.h
+++ b/buffet/storage_impls.h
@@ -19,7 +19,7 @@
explicit FileStorage(const base::FilePath& file_path);
virtual ~FileStorage() = default;
std::unique_ptr<base::Value> Load() override;
- bool Save(const base::Value* config) override;
+ bool Save(const base::Value& config) override;
private:
base::FilePath file_path_;
@@ -32,7 +32,7 @@
MemStorage() = default;
virtual ~MemStorage() = default;
std::unique_ptr<base::Value> Load() override;
- bool Save(const base::Value* config) override;
+ bool Save(const base::Value& config) override;
int save_count() { return save_count_; }
void reset_save_count() { save_count_ = 0; }
diff --git a/buffet/storage_interface.h b/buffet/storage_interface.h
index 26d71c1..4f05235 100644
--- a/buffet/storage_interface.h
+++ b/buffet/storage_interface.h
@@ -24,7 +24,7 @@
// If saved successfully, returns true. Could fail when writing to
// physical storage like file system for various reasons (out of disk space,
// access permissions, etc).
- virtual bool Save(const base::Value* config) = 0;
+ virtual bool Save(const base::Value& config) = 0;
};
} // namespace buffet
