PolicyManager: Move payload_size to int64_t.
Code style encourages signed types instead of unsigned types and the
value being exposed is already signed. This patch fixes that for the
payload_size variable and adapts the BoxedValue::ValuePrinter
implementations to use the int64_t and uint64_t types.
BUG=None
TEST=Unit tests updated
Change-Id: I21310c59d8c2654c43cac27265055c8577341562
Reviewed-on: https://chromium-review.googlesource.com/198269
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/policy_manager/boxed_value.cc b/policy_manager/boxed_value.cc
index 0cf7ed2..02bc0bd 100644
--- a/policy_manager/boxed_value.cc
+++ b/policy_manager/boxed_value.cc
@@ -28,31 +28,31 @@
}
template<>
+string BoxedValue::ValuePrinter<int>(const void *value) {
+ const int* val = reinterpret_cast<const int*>(value);
+ return base::IntToString(*val);
+}
+
+template<>
string BoxedValue::ValuePrinter<unsigned int>(const void *value) {
const unsigned int* val = reinterpret_cast<const unsigned int*>(value);
return base::UintToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<unsigned long>(const void *value) {
- const unsigned long* val = reinterpret_cast<const unsigned long*>(value);
- return base::Uint64ToString(static_cast<uint64>(*val));
+string BoxedValue::ValuePrinter<int64_t>(const void *value) {
+ const int64_t* val = reinterpret_cast<const int64_t*>(value);
+ return base::Int64ToString(*val);
}
template<>
-string BoxedValue::ValuePrinter<unsigned long long>(const void *value) {
- const unsigned long long* val =
- reinterpret_cast<const unsigned long long*>(value);
+string BoxedValue::ValuePrinter<uint64_t>(const void *value) {
+ const uint64_t* val =
+ reinterpret_cast<const uint64_t*>(value);
return base::Uint64ToString(static_cast<uint64>(*val));
}
template<>
-string BoxedValue::ValuePrinter<int>(const void *value) {
- const int* val = reinterpret_cast<const int*>(value);
- return base::IntToString(*val);
-}
-
-template<>
string BoxedValue::ValuePrinter<bool>(const void *value) {
const bool* val = reinterpret_cast<const bool*>(value);
return *val ? "true" : "false";
diff --git a/policy_manager/boxed_value_unittest.cc b/policy_manager/boxed_value_unittest.cc
index 6319ba7..a84604d 100644
--- a/policy_manager/boxed_value_unittest.cc
+++ b/policy_manager/boxed_value_unittest.cc
@@ -102,19 +102,21 @@
EXPECT_EQ("42", BoxedValue(new int(42)).ToString());
}
+TEST(PmBoxedValueTest, Int64ToString) {
+ // -123456789012345 doensn't fit in 32-bit integers.
+ EXPECT_EQ("-123456789012345", BoxedValue(
+ new int64_t(-123456789012345LL)).ToString());
+}
+
TEST(PmBoxedValueTest, UnsignedIntToString) {
// 4294967295 is the biggest possible 32-bit unsigned integer.
- EXPECT_EQ("4294967295", BoxedValue(new unsigned int(4294967295)).ToString());
+ EXPECT_EQ("4294967295", BoxedValue(new unsigned int(4294967295U)).ToString());
}
-TEST(PmBoxedValueTest, UnsignedLongToString) {
- EXPECT_EQ("4294967295", BoxedValue(new unsigned long(4294967295)).ToString());
-}
-
-TEST(PmBoxedValueTest, UnsignedLongLongToString) {
+TEST(PmBoxedValueTest, UnsignedInt64ToString) {
// 18446744073709551615 is the biggest possible 64-bit unsigned integer.
EXPECT_EQ("18446744073709551615", BoxedValue(
- new unsigned long long(18446744073709551615ULL)).ToString());
+ new uint64_t(18446744073709551615ULL)).ToString());
}
TEST(PmBoxedValueTest, BoolToString) {
diff --git a/policy_manager/fake_updater_provider.h b/policy_manager/fake_updater_provider.h
index c9fbdc6..e8fee63 100644
--- a/policy_manager/fake_updater_provider.h
+++ b/policy_manager/fake_updater_provider.h
@@ -41,7 +41,7 @@
return &var_new_version_;
}
- virtual FakeVariable<size_t>* var_payload_size() override {
+ virtual FakeVariable<int64_t>* var_payload_size() override {
return &var_payload_size_;
}
@@ -76,7 +76,7 @@
FakeVariable<double> var_progress_{"progress", kVariableModePoll};
FakeVariable<Stage> var_stage_{"stage", kVariableModePoll};
FakeVariable<std::string> var_new_version_{"new_version", kVariableModePoll};
- FakeVariable<size_t> var_payload_size_{"payload_size", kVariableModePoll};
+ FakeVariable<int64_t> var_payload_size_{"payload_size", kVariableModePoll};
FakeVariable<std::string> var_curr_channel_{
"curr_channel", kVariableModePoll};
FakeVariable<std::string> var_new_channel_{"new_channel", kVariableModePoll};
diff --git a/policy_manager/real_updater_provider.cc b/policy_manager/real_updater_provider.cc
index bc1350b..4455e7f 100644
--- a/policy_manager/real_updater_provider.cc
+++ b/policy_manager/real_updater_provider.cc
@@ -180,12 +180,12 @@
};
// A variable reporting the size of the update being processed in bytes.
-class PayloadSizeVariable : public UpdaterVariableBase<size_t> {
+class PayloadSizeVariable : public UpdaterVariableBase<int64_t> {
public:
- using UpdaterVariableBase<size_t>::UpdaterVariableBase;
+ using UpdaterVariableBase<int64_t>::UpdaterVariableBase;
private:
- virtual const size_t* GetValue(TimeDelta /* timeout */,
+ virtual const int64_t* GetValue(TimeDelta /* timeout */,
string* errmsg) override {
GetStatusHelper raw(system_state(), errmsg);
if (!raw.is_success())
@@ -197,7 +197,7 @@
return NULL;
}
- return new size_t(static_cast<size_t>(raw.payload_size()));
+ return new int64_t(raw.payload_size());
}
DISALLOW_COPY_AND_ASSIGN(PayloadSizeVariable);
diff --git a/policy_manager/real_updater_provider.h b/policy_manager/real_updater_provider.h
index 22b9e0f..7ed0b4e 100644
--- a/policy_manager/real_updater_provider.h
+++ b/policy_manager/real_updater_provider.h
@@ -51,7 +51,7 @@
return var_new_version_.get();
}
- virtual Variable<size_t>* var_payload_size() override {
+ virtual Variable<int64_t>* var_payload_size() override {
return var_payload_size_.get();
}
@@ -87,7 +87,7 @@
scoped_ptr<Variable<double>> var_progress_;
scoped_ptr<Variable<Stage>> var_stage_;
scoped_ptr<Variable<std::string>> var_new_version_;
- scoped_ptr<Variable<size_t>> var_payload_size_;
+ scoped_ptr<Variable<int64_t>> var_payload_size_;
scoped_ptr<Variable<std::string>> var_curr_channel_;
scoped_ptr<Variable<std::string>> var_new_channel_;
scoped_ptr<Variable<bool>> var_p2p_enabled_;
diff --git a/policy_manager/real_updater_provider_unittest.cc b/policy_manager/real_updater_provider_unittest.cc
index dd6fb57..7e2be74 100644
--- a/policy_manager/real_updater_provider_unittest.cc
+++ b/policy_manager/real_updater_provider_unittest.cc
@@ -303,7 +303,7 @@
EXPECT_CALL(*fake_sys_state_.mock_update_attempter(),
GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(0)), Return(true)));
- PmTestUtils::ExpectVariableHasValue(static_cast<size_t>(0),
+ PmTestUtils::ExpectVariableHasValue(static_cast<int64_t>(0),
provider_->var_payload_size());
}
@@ -312,7 +312,7 @@
GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(567890)),
Return(true)));
- PmTestUtils::ExpectVariableHasValue(static_cast<size_t>(567890),
+ PmTestUtils::ExpectVariableHasValue(static_cast<int64_t>(567890),
provider_->var_payload_size());
}
@@ -321,7 +321,7 @@
GetStatus(_, _, _, _, _))
.WillOnce(DoAll(SetArgPointee<4>(static_cast<int64_t>(1) << 31),
Return(true)));
- PmTestUtils::ExpectVariableHasValue(static_cast<size_t>(1) << 31,
+ PmTestUtils::ExpectVariableHasValue(static_cast<int64_t>(1) << 31,
provider_->var_payload_size());
}
diff --git a/policy_manager/updater_provider.h b/policy_manager/updater_provider.h
index fa5f48a..ff76cce 100644
--- a/policy_manager/updater_provider.h
+++ b/policy_manager/updater_provider.h
@@ -59,8 +59,9 @@
// A variable returning the update target version.
virtual Variable<std::string>* var_new_version() = 0;
- // A variable returning the update payload size.
- virtual Variable<size_t>* var_payload_size() = 0;
+ // A variable returning the update payload size. The payload size is
+ // guaranteed to be non-negative.
+ virtual Variable<int64_t>* var_payload_size() = 0;
// A variable returning the current channel.
virtual Variable<std::string>* var_curr_channel() = 0;