Commit fcd54faf authored by Daniel Erat's avatar Daniel Erat Committed by Commit Bot

chromeos: Drop "(error)" values from crossystem.

Update StatisticsProvider to drop pairs received from
crossystem that have "(error)" values. NameValuePairsParser
ignores duplicate values, so we should get rid of these so
they don't prevent us from saving real values later.

Also downgrade a "Statistics loaded after waiting ___ ms"
LOG(WARNING) to VLOG(1), since this seems to be the common
case on a caroline ToT device and it just ends up cluttering
the logs.

Bug: 844258
Change-Id: Idf869617a03992e51a720359b4ebfdac91dec6e9
Reviewed-on: https://chromium-review.googlesource.com/1072706Reviewed-by: default avatarThiemo Nagel <tnagel@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563053}
parent 3684f1d6
...@@ -73,6 +73,17 @@ bool NameValuePairsParser::ParseNameValuePairsFromTool( ...@@ -73,6 +73,17 @@ bool NameValuePairsParser::ParseNameValuePairsFromTool(
comment_delim); comment_delim);
} }
void NameValuePairsParser::DeletePairsWithValue(const std::string& value) {
auto it = map_->begin();
while (it != map_->end()) {
if (it->second == value) {
it = map_->erase(it);
} else {
it++;
}
}
}
void NameValuePairsParser::AddNameValuePair(const std::string& key, void NameValuePairsParser::AddNameValuePair(const std::string& key,
const std::string& value) { const std::string& value) {
const auto it = map_->find(key); const auto it = map_->find(key);
......
...@@ -43,6 +43,9 @@ class CHROMEOS_EXPORT NameValuePairsParser { ...@@ -43,6 +43,9 @@ class CHROMEOS_EXPORT NameValuePairsParser {
const std::string& delim, const std::string& delim,
const std::string& comment_delim); const std::string& comment_delim);
// Delete all pairs with |value|.
void DeletePairsWithValue(const std::string& value);
private: private:
FRIEND_TEST_ALL_PREFIXES(NameValuePairsParser, TestParseNameValuePairs); FRIEND_TEST_ALL_PREFIXES(NameValuePairsParser, TestParseNameValuePairs);
FRIEND_TEST_ALL_PREFIXES(NameValuePairsParser, FRIEND_TEST_ALL_PREFIXES(NameValuePairsParser,
......
...@@ -104,5 +104,16 @@ TEST(NameValuePairsParser, TestParseNameValuePairsFromTool) { ...@@ -104,5 +104,16 @@ TEST(NameValuePairsParser, TestParseNameValuePairsFromTool) {
map["vdat_timers"]); map["vdat_timers"]);
} }
TEST(NameValuePairsParser, DeletePairsWithValue) {
NameValuePairsParser::NameValueMap map = {
{"foo", "good"}, {"bar", "bad"}, {"baz", "good"}, {"end", "bad"},
};
NameValuePairsParser parser(&map);
parser.DeletePairsWithValue("bad");
ASSERT_EQ(2u, map.size());
EXPECT_EQ("good", map["foo"]);
EXPECT_EQ("good", map["baz"]);
}
} // namespace system } // namespace system
} // namespace chromeos } // namespace chromeos
...@@ -327,8 +327,8 @@ bool StatisticsProviderImpl::WaitForStatisticsLoaded() { ...@@ -327,8 +327,8 @@ bool StatisticsProviderImpl::WaitForStatisticsLoaded() {
base::TimeDelta dtime = base::Time::Now() - start_time; base::TimeDelta dtime = base::Time::Now() - start_time;
if (statistics_loaded_.IsSignaled()) { if (statistics_loaded_.IsSignaled()) {
LOG(WARNING) << "Statistics loaded after waiting " VLOG(1) << "Statistics loaded after waiting " << dtime.InMilliseconds()
<< dtime.InMilliseconds() << "ms."; << "ms.";
return true; return true;
} }
...@@ -524,6 +524,9 @@ void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) { ...@@ -524,6 +524,9 @@ void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) {
kCrosSystemCommentDelim)) { kCrosSystemCommentDelim)) {
LOG(ERROR) << "Errors parsing output from: " << kCrosSystemTool; LOG(ERROR) << "Errors parsing output from: " << kCrosSystemTool;
} }
// Drop useless "(error)" values so they don't displace valid values
// supplied later by other tools: https://crbug.com/844258
parser.DeletePairsWithValue(kCrosSystemValueError);
} }
base::FilePath machine_info_path; base::FilePath machine_info_path;
...@@ -569,11 +572,8 @@ void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) { ...@@ -569,11 +572,8 @@ void StatisticsProviderImpl::LoadMachineStatistics(bool load_oem_manifest) {
// Ensure that the hardware class key is present with the expected // Ensure that the hardware class key is present with the expected
// key name, and if it couldn't be retrieved, that the value is "unknown". // key name, and if it couldn't be retrieved, that the value is "unknown".
std::string hardware_class = machine_info_[kHardwareClassCrosSystemKey]; std::string hardware_class = machine_info_[kHardwareClassCrosSystemKey];
if (hardware_class.empty() || hardware_class == kCrosSystemValueError) { machine_info_[kHardwareClassKey] =
machine_info_[kHardwareClassKey] = kHardwareClassValueUnknown; !hardware_class.empty() ? hardware_class : kHardwareClassValueUnknown;
} else {
machine_info_[kHardwareClassKey] = hardware_class;
}
if (base::SysInfo::IsRunningOnChromeOS()) { if (base::SysInfo::IsRunningOnChromeOS()) {
// By default, assume that this is *not* a VM. If crossystem is not present, // By default, assume that this is *not* a VM. If crossystem is not present,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment