thermald: base::Value refactor in ConfigParser
Replace base::{Dictionary,List}Value by dictionary or list type
base::Value.
In particular this removes base::Value::GetAs* which would be removed
after libchrome r920000 uprev.
BUG=chromium:1099111, b:198993705
TEST=FEATURES=test emerge-gale thermald
Change-Id: I18b348bd2d91ca8219b2e2df1738a4222c83eafb
Reviewed-on: https://chrome-internal-review.googlesource.com/c/chromeos/thermald/+/4108491
Tested-by: Grace Cham <hscham@google.com>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Grace Cham <hscham@google.com>
diff --git a/config_parser.cc b/config_parser.cc
index 1e47f6f..6f238de 100644
--- a/config_parser.cc
+++ b/config_parser.cc
@@ -35,20 +35,19 @@
return nullptr;
}
- base::Optional<base::Value> root_node_value(
+ base::Optional<base::Value> root_node(
base::JSONReader::Read(data, base::JSON_ALLOW_TRAILING_COMMAS));
- if (!root_node_value) {
+ if (!root_node) {
LOG(ERROR) << "Configuration error: Configuration file is invalid";
return nullptr;
}
ConfigParser parser;
- return parser.ParseConfiguration(&root_node_value.value());
+ return parser.ParseConfiguration(root_node.value());
}
-Configuration *ConfigParser::ParseConfiguration(base::Value *root_node_value) {
- base::DictionaryValue *root_node;
- if (!root_node_value->GetAsDictionary(&root_node)) {
+Configuration *ConfigParser::ParseConfiguration(const base::Value &root_node) {
+ if (!root_node.is_dict()) {
LOG(ERROR) << "Configuration error: Configuration file is invalid";
return nullptr;
}
@@ -56,32 +55,33 @@
unique_ptr<Configuration> cfg(new Configuration);
cfg_ = cfg.get();
- base::ListValue *sensors;
- if (!root_node->GetList("sensors", &sensors)) {
+ const base::Value *sensors = root_node.FindListKey("sensors");
+ if (!sensors) {
LOG(ERROR) << "Configuration error: Sensor configuration missing";
return nullptr;
}
- if (!ProcessSensorConfig(sensors)) {
+ if (!ProcessSensorConfig(*sensors)) {
return nullptr;
}
- base::ListValue *thermal_zones;
- if (!root_node->GetList("thermal_zones", &thermal_zones)) {
+ const base::Value *thermal_zones = root_node.FindListKey("thermal_zones");
+ if (!thermal_zones) {
LOG(ERROR) << "Configuration error: Thermal zone configuration missing";
return nullptr;
}
- if (!ProcessThermalZoneConfig(thermal_zones)) {
+ if (!ProcessThermalZoneConfig(*thermal_zones)) {
return nullptr;
}
// If uma_metric_prefix section is not present in thermal configuration,
// then default UMA metric prefix will be "Platform".
cfg_->uma_metric_prefix = "Platform";
- base::ListValue *uma_metric_prefix;
- if (root_node->GetList("uma_metric_prefix", &uma_metric_prefix)) {
- if (!ProcessUmaMetricPrefixConfig(uma_metric_prefix)) {
+ const base::Value *uma_metric_prefix =
+ root_node.FindListKey("uma_metric_prefix");
+ if (uma_metric_prefix) {
+ if (!ProcessUmaMetricPrefixConfig(*uma_metric_prefix)) {
return nullptr;
}
}
@@ -90,21 +90,20 @@
}
bool ConfigParser::ProcessOneThermalState(ThermalZoneCfg *zone_cfg,
- base::ListValue *state_node,
+ const base::Value &state_node,
int index_state) {
unique_ptr<ThermalStateCfg> state_cfg(new ThermalStateCfg);
- base::ListValue *threshold_list;
- base::DictionaryValue *output_dict;
- if ((state_node->GetSize() != 3) ||
- !state_node->GetInteger(0, &state_cfg->id) ||
- !state_node->GetList(1, &threshold_list) ||
- !state_node->GetDictionary(2, &output_dict)) {
+ const auto &state_list = state_node.GetList();
+ if ((state_list.size() != 3) || !state_list[0].is_int() ||
+ !state_list[1].is_list() || !state_list[2].is_dict()) {
LOG(ERROR) << "Configuration error: State " << index_state
<< " in thermal zone '" << zone_cfg->name << "' is invalid";
return false;
}
+ state_cfg->id = state_list[0].GetInt();
+ const auto &threshold_list = state_list[1].GetList();
- const size_t num_thresholds = threshold_list->GetSize();
+ const size_t num_thresholds = threshold_list.size();
if (((index_state == 0) && (num_thresholds != 0)) ||
((index_state != 0) && (num_thresholds != zone_cfg->sensors.size()))) {
LOG(ERROR) << "Configuration error: Threshold configuration of state "
@@ -114,24 +113,27 @@
}
if (index_state != 0) {
- for (size_t i = 0; i < num_thresholds; i++) {
- base::ListValue *thresholds_cfg;
- if (!threshold_list->GetList(i, &thresholds_cfg)) {
+ size_t i = 0;
+ for (const auto &thresholds_cfg : threshold_list) {
+ if (!thresholds_cfg.is_list()) {
LOG(ERROR) << "Configuration error: Threshold configuration of state "
<< state_cfg->id << " in thermal zone '" << zone_cfg->name
<< "' is invalid";
return false;
}
- ThermalPointThresholds thresholds;
- if ((thresholds_cfg->GetSize() != 2) ||
- !thresholds_cfg->GetInteger(0, &thresholds.activation) ||
- !thresholds_cfg->GetInteger(1, &thresholds.bottom)) {
+ const auto &thresholds_cfg_list = thresholds_cfg.GetList();
+ if ((thresholds_cfg_list.size() != 2) ||
+ !thresholds_cfg_list[0].is_int() ||
+ !thresholds_cfg_list[1].is_int()) {
LOG(ERROR) << "Configuration error: Threshold configuration of state "
<< state_cfg->id << " in thermal zone '" << zone_cfg->name
<< "' is invalid";
return false;
}
+ ThermalPointThresholds thresholds;
+ thresholds.activation = thresholds_cfg_list[0].GetInt();
+ thresholds.bottom = thresholds_cfg_list[1].GetInt();
if ((thresholds.activation != kNoThreshold) &&
(thresholds.bottom != kNoThreshold)) {
@@ -163,6 +165,7 @@
}
state_cfg->thresholds.insert(make_pair(zone_cfg->sensors[i], thresholds));
+ i++;
}
} else {
// Set dummy thresholds for first state.
@@ -179,16 +182,15 @@
}
}
- for (base::DictionaryValue::Iterator it(*output_dict); !it.IsAtEnd();
- it.Advance()) {
- int value;
- if (!it.value().GetAsInteger(&value)) {
+ for (const auto &kv : state_list[2].DictItems()) {
+ auto value = kv.second.GetIfInt();
+ if (!value.has_value()) {
LOG(ERROR) << "Configuration error: Thermal output configuration "
<< "of state " << state_cfg->id << " in thermal zone '"
<< zone_cfg->name << "' is invalid";
return false;
}
- ThermalStateOutput output = {it.key(), value};
+ ThermalStateOutput output = {kv.first, *value};
state_cfg->outputs.push_back(output);
}
@@ -197,35 +199,35 @@
return true;
}
-bool ConfigParser::ProcessOneThermalZone(base::ListValue *zone_node,
+bool ConfigParser::ProcessOneThermalZone(const base::Value &zone_node,
int index_zone) {
unique_ptr<ThermalZoneCfg> zone_cfg(new ThermalZoneCfg);
- base::ListValue *sensors;
- base::ListValue *thermal_states;
- if ((zone_node->GetSize() != 3) ||
- !zone_node->GetString(0, &zone_cfg->name) ||
- !zone_node->GetList(1, &sensors) ||
- !zone_node->GetList(2, &thermal_states)) {
+ const auto &zone_list = zone_node.GetList();
+ if ((zone_list.size() != 3) || !zone_list[0].is_string() ||
+ !zone_list[1].is_list() || !zone_list[2].is_list()) {
LOG(ERROR) << "Configuration error: Configuration of thermal zone "
<< index_zone << " is invalid";
return false;
}
+ zone_cfg->name = zone_list[0].GetString();
+ const auto &sensors = zone_list[1].GetList();
+ const auto &thermal_states = zone_list[2].GetList();
- const size_t num_sensors = sensors->GetSize();
+ const size_t num_sensors = sensors.size();
if (num_sensors == 0) {
LOG(ERROR) << "Configuration error: Thermal zone '" << zone_cfg->name
<< "' has no sensors";
return false;
}
- for (size_t i = 0; i < num_sensors; i++) {
- string sensor_name;
- if (!sensors->GetString(i, &sensor_name)) {
+ for (const auto &sensor : sensors) {
+ if (!sensor.is_string()) {
LOG(ERROR) << "Configuration error: Sensor configuration of << "
<< "thermal zone '" << zone_cfg->name << "' is invalid";
return false;
}
+ const string& sensor_name = sensor.GetString();
if (cfg_->sensors.find(sensor_name) == cfg_->sensors.end()) {
LOG(ERROR) << "Configuration error: Thermal zone '" << zone_cfg->name
@@ -236,24 +238,25 @@
zone_cfg->sensors.push_back(sensor_name);
}
- const size_t num_states = thermal_states->GetSize();
+ const size_t num_states = thermal_states.size();
if (num_states < 2) {
LOG(ERROR) << "Configuration error: Not enough thermal states in "
<< "thermal zone '" << zone_cfg->name << "'";
return false;
}
- for (size_t i = 0; i < num_states; i++) {
- base::ListValue *state_node;
- if (!thermal_states->GetList(i, &state_node)) {
+ size_t i = 0;
+ for (const auto &state : thermal_states) {
+ if (!state.is_list()) {
LOG(ERROR) << "Configuration error: State " << i << " in thermal zone '"
<< zone_cfg->name << "' is invalid";
return false;
}
- if (!ProcessOneThermalState(zone_cfg.get(), state_node, i)) {
+ if (!ProcessOneThermalState(zone_cfg.get(), state, i)) {
return false;
}
+ i++;
}
cfg_->zones.insert(make_pair(zone_cfg->name, std::move(zone_cfg)));
@@ -261,10 +264,10 @@
return true;
}
-bool ConfigParser::ProcessThermalZoneConfig(base::ListValue *thermal_zones) {
- for (size_t i = 0; i < thermal_zones->GetSize(); i++) {
- base::ListValue *zone_node;
- if (!thermal_zones->GetList(i, &zone_node)) {
+bool ConfigParser::ProcessThermalZoneConfig(const base::Value &thermal_zones) {
+ size_t i = 0;
+ for (const auto &zone_node : thermal_zones.GetList()) {
+ if (!zone_node.is_list()) {
LOG(ERROR) << "Configuration error: Configuration of thermal zone "
<< i << " is invalid";
return false;
@@ -273,26 +276,28 @@
if (!ProcessOneThermalZone(zone_node, i)) {
return false;
}
+ i++;
}
return true;
}
-bool ConfigParser::ProcessOneSensor(base::ListValue *sensor_node,
+bool ConfigParser::ProcessOneSensor(const base::Value &sensor_node,
int index_sensor) {
unique_ptr<SensorCfg> sensor_cfg(new SensorCfg);
- string sensor_type;
- base::ListValue *params;
- if ((sensor_node->GetSize() != 4) ||
- !sensor_node->GetString(0, &sensor_cfg->name) ||
- !sensor_node->GetString(1, &sensor_type) ||
- !sensor_node->GetList(2, ¶ms) ||
- !sensor_node->GetInteger(3, &sensor_cfg->sampling_period)) {
+ const auto &sensor_node_list = sensor_node.GetList();
+ if ((sensor_node_list.size() != 4) || !sensor_node_list[0].is_string() ||
+ !sensor_node_list[1].is_string() || !sensor_node_list[2].is_list() ||
+ !sensor_node_list[3].is_int()) {
LOG(ERROR) << "Configuration error: Configuration of sensor "
<< index_sensor << " is invalid";
return false;
}
+ sensor_cfg->name = sensor_node_list[0].GetString();
+ const string& sensor_type = sensor_node_list[1].GetString();
+ const auto ¶ms = sensor_node_list[2].GetList();
+ sensor_cfg->sampling_period = sensor_node_list[3].GetInt();
if (cfg_->sensors.find(sensor_cfg->name) != cfg_->sensors.end()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
@@ -301,59 +306,53 @@
}
if (sensor_type == "ath10k") {
- string wlan_interface;
- if ((params->GetSize() != 1) ||
- (!params->GetString(0, &wlan_interface))) {
+ if ((params.size() != 1) || !params[0].is_string()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
<< "' has invalid parameters";
return false;
}
+ const string& wlan_interface = params[0].GetString();
base::strlcpy(sensor_cfg->ath10k_sensor.wlan_interface,
wlan_interface.c_str(),
sizeof(sensor_cfg->ath10k_sensor.wlan_interface));
sensor_cfg->type = kAth10kSensor;
} else if (sensor_type == "hwmon") {
- if ((params->GetSize() != 2) ||
- (!params->GetInteger(0, &sensor_cfg->hwmon_sensor.chip_id)) ||
- (!params->GetInteger(1, &sensor_cfg->hwmon_sensor.sensor_id))) {
+ if ((params.size() != 2) || !params[0].is_int() || !params[1].is_int()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
<< "' has invalid parameters";
return false;
}
+ sensor_cfg->hwmon_sensor.chip_id = params[0].GetInt();
+ sensor_cfg->hwmon_sensor.sensor_id = params[1].GetInt();
sensor_cfg->type = kHwmonSensor;
} else if (sensor_type == "thermal_zone") {
- if ((params->GetSize() != 1) ||
- (!params->GetInteger(0, &sensor_cfg->thermal_zone_sensor.zone_id))) {
+ if ((params.size() != 1) || !params[0].is_int()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
<< "' has invalid parameters";
return false;
}
+ sensor_cfg->thermal_zone_sensor.zone_id = params[0].GetInt();
sensor_cfg->type = kThermalZoneSensor;
} else if (sensor_type == "iio") {
- string iio_device_name;
- string iio_sensor_name;
- if ((params->GetSize() != 2) ||
- (!params->GetString(0, &iio_device_name)) ||
- (!params->GetString(1, &iio_sensor_name))) {
+ if ((params.size() != 2) || !params[0].is_string() ||
+ !params[1].is_string()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
<< "' has invalid parameters";
return false;
}
base::strlcpy(sensor_cfg->iio_sensor.iio_device_name,
- iio_device_name.c_str(),
+ params[0].GetString().c_str(),
sizeof(sensor_cfg->iio_sensor.iio_device_name));
base::strlcpy(sensor_cfg->iio_sensor.iio_sensor_name,
- iio_sensor_name.c_str(),
+ params[1].GetString().c_str(),
sizeof(sensor_cfg->iio_sensor.iio_sensor_name));
sensor_cfg->type = kIioSensor;
} else if (sensor_type == "fake") {
- string fake_data_file;
- if ((params->GetSize() != 1) ||
- (!params->GetString(0, &fake_data_file))) {
+ if ((params.size() != 1) || !params[0].is_string()) {
LOG(ERROR) << "Configuration error: Sensor '" << sensor_cfg->name
<< "' has invalid parameters";
return false;
@@ -361,7 +360,8 @@
// This might cause a memory leak, but as this is a fake sensor it is
// preferrable over reserving memory for the path in SensorCfg
- sensor_cfg->fake_sensor.fake_data_file = strdup(fake_data_file.c_str());
+ sensor_cfg->fake_sensor.fake_data_file =
+ strdup(params[0].GetString().c_str());
sensor_cfg->type = kFakeSensor;
} else {
@@ -375,10 +375,10 @@
return true;
}
-bool ConfigParser::ProcessSensorConfig(base::ListValue *sensors) {
- for (size_t i = 0; i < sensors->GetSize(); i++) {
- base::ListValue *sensor_node;
- if (!sensors->GetList(i, &sensor_node)) {
+bool ConfigParser::ProcessSensorConfig(const base::Value &sensors) {
+ size_t i = 0;
+ for (const auto &sensor_node : sensors.GetList()) {
+ if (!sensor_node.is_list()) {
LOG(ERROR) << "Configuration error: Configuration of sensor "
<< i << " is invalid";
return false;
@@ -387,31 +387,34 @@
if (!ProcessOneSensor(sensor_node, i)) {
return false;
}
+ i++;
}
return true;
}
bool ConfigParser::ProcessUmaMetricPrefixConfig(
- base::ListValue *uma_metric_prefix) {
- if(uma_metric_prefix->GetSize() != 1) {
+ const base::Value &uma_metric_prefix) {
+ const auto &prefix_list = uma_metric_prefix.GetList();
+ if (prefix_list.size() != 1) {
LOG(ERROR) << "Configuration error: Configuration of uma_metric_prefix "
<< "is invalid";
return false;
}
- base::ListValue *params;
- if (!uma_metric_prefix->GetList(0, ¶ms)) {
+ if (!prefix_list[0].is_list()) {
LOG(ERROR) << "Configuration error: uma_metric_prefix has"
<< " invalid parameters";
return false;
}
+ const auto ¶ms = prefix_list[0].GetList();
- if (!params->GetString(0, &cfg_->uma_metric_prefix)) {
+ if (!params[0].is_string()) {
LOG(ERROR) << "Configuration error: Configuration of uma_metric_prefix"
<< " is invalid";
return false;
}
+ cfg_->uma_metric_prefix = params[0].GetString();
return true;
}
diff --git a/config_parser.h b/config_parser.h
index c7200c6..6f4d0f1 100644
--- a/config_parser.h
+++ b/config_parser.h
@@ -78,15 +78,14 @@
private:
ConfigParser() {}
- Configuration *ParseConfiguration(base::Value *root_node_value);
- // TODO(crbug.com/1099111): change base::ListValue* to const base::Value&
- bool ProcessSensorConfig(base::ListValue *sensors);
- bool ProcessOneSensor(base::ListValue *sensor_node, int index_sensor);
- bool ProcessThermalZoneConfig(base::ListValue *thermal_zones);
- bool ProcessOneThermalZone(base::ListValue *thermal_zone, int index_zone);
+ Configuration *ParseConfiguration(const base::Value &root_node_value);
+ bool ProcessSensorConfig(const base::Value &sensors);
+ bool ProcessOneSensor(const base::Value &sensor_node, int index_sensor);
+ bool ProcessThermalZoneConfig(const base::Value &thermal_zones);
+ bool ProcessOneThermalZone(const base::Value &thermal_zone, int index_zone);
bool ProcessOneThermalState(ThermalZoneCfg *zone_cfg,
- base::ListValue *state_node, int index_state);
- bool ProcessUmaMetricPrefixConfig(base::ListValue *uma_metric_prefix);
+ const base::Value &state_node, int index_state);
+ bool ProcessUmaMetricPrefixConfig(const base::Value &uma_metric_prefix);
struct Configuration *cfg_;
std::vector<ThermalPointThresholds> thresholds_prev_state_;
