Merge "Perfprofd: Fix script dependency"
diff --git a/perfprofd/binder_interface/perfprofd_binder.cc b/perfprofd/binder_interface/perfprofd_binder.cc
index 0976b7b..a00ca14 100644
--- a/perfprofd/binder_interface/perfprofd_binder.cc
+++ b/perfprofd/binder_interface/perfprofd_binder.cc
@@ -159,9 +159,11 @@
// Split configuration along colon.
std::vector<std::string> args = base::Split(String8(config).string(), ":");
for (auto& arg : args) {
- if (!reader.Read(arg, /* fail_on_error */ true)) {
- error_msg = base::StringPrintf("Could not parse %s", arg.c_str());
- return Status::fromExceptionCode(1, error_msg.c_str());
+ if (!reader.Read(arg, /* fail_on_error */ true, &error_msg)) {
+ std::string tmp = base::StringPrintf("Could not parse %s: %s",
+ arg.c_str(),
+ error_msg.c_str());
+ return Status::fromExceptionCode(1, tmp.c_str());
}
}
auto config_fn = [&](ThreadedConfig& config) {
@@ -281,8 +283,9 @@
} else if (args[0] == String16("startProfiling")) {
ConfigReader reader;
for (size_t i = 1; i < args.size(); ++i) {
- if (!reader.Read(String8(args[i]).string(), /* fail_on_error */ true)) {
- err_str << base::StringPrintf("Could not parse %s", String8(args[i]).string())
+ std::string error_msg;
+ if (!reader.Read(String8(args[i]).string(), /* fail_on_error */ true, &error_msg)) {
+ err_str << "Could not parse '" << String8(args[i]).string() << "': " << error_msg
<< std::endl;
return BAD_VALUE;
}
diff --git a/perfprofd/configreader.cc b/perfprofd/configreader.cc
index 842adcb..d51d078 100644
--- a/perfprofd/configreader.cc
+++ b/perfprofd/configreader.cc
@@ -15,18 +15,22 @@
** limitations under the License.
*/
-#include <assert.h>
-#include <stdio.h>
-#include <stdlib.h>
+#include "configreader.h"
+
+#include <inttypes.h>
#include <algorithm>
+#include <climits>
+#include <cstdlib>
#include <cstring>
#include <sstream>
#include <android-base/file.h>
#include <android-base/logging.h>
+#include <android-base/parseint.h>
+#include <android-base/stringprintf.h>
-#include "configreader.h"
+using android::base::StringPrintf;
//
// Config file path
@@ -217,36 +221,45 @@
// warnings or errors to the system logs if the line can't be
// interpreted properly.
//
-bool ConfigReader::parseLine(const char *key,
- const char *value,
- unsigned linecount)
+bool ConfigReader::parseLine(const std::string& key,
+ const std::string& value,
+ unsigned linecount,
+ std::string* error_msg)
{
- assert(key);
- assert(value);
+ if (key.empty()) {
+ *error_msg = StringPrintf("line %u: Key is empty", linecount);
+ return false;
+ }
+ if (value.empty()) {
+ *error_msg = StringPrintf("line %u: Value for %s is empty", linecount, key.c_str());
+ return false;
+ }
auto uit = u_entries.find(key);
if (uit != u_entries.end()) {
- unsigned uvalue = 0;
- if (isdigit(value[0]) == 0 || sscanf(value, "%u", &uvalue) != 1) {
- LOG(WARNING) << "line " << linecount << ": malformed unsigned value (ignored)";
+ uint64_t conv;
+ if (!android::base::ParseUint(value, &conv)) {
+ *error_msg = StringPrintf("line %u: value %s cannot be parsed", linecount, value.c_str());
+ }
+ values vals;
+ auto iit = u_info.find(key);
+ DCHECK(iit != u_info.end());
+ vals = iit->second;
+ if (conv < vals.minv || conv > vals.maxv) {
+ *error_msg = StringPrintf("line %u: "
+ "specified value %" PRIu64 " for '%s' "
+ "outside permitted range [%u %u]",
+ linecount,
+ conv,
+ key.c_str(),
+ vals.minv,
+ vals.maxv);
return false;
} else {
- values vals;
- auto iit = u_info.find(key);
- assert(iit != u_info.end());
- vals = iit->second;
- if (uvalue < vals.minv || uvalue > vals.maxv) {
- LOG(WARNING) << "line " << linecount << ": "
- << "specified value " << uvalue << " for '" << key << "' "
- << "outside permitted range [" << vals.minv << " " << vals.maxv
- << "] (ignored)";
- return false;
- } else {
- if (trace_config_read) {
- LOG(INFO) << "option " << key << " set to " << uvalue;
- }
- uit->second = uvalue;
+ if (trace_config_read) {
+ LOG(INFO) << "option " << key << " set to " << conv;
}
+ uit->second = static_cast<unsigned>(conv);
}
trace_config_read = (getUnsignedValue("trace_config_read") != 0);
return true;
@@ -261,7 +274,7 @@
return true;
}
- LOG(WARNING) << "line " << linecount << ": unknown option '" << key << "' ignored";
+ *error_msg = StringPrintf("line %u: unknown option '%s'", linecount, key.c_str());
return false;
}
@@ -279,12 +292,30 @@
if (! android::base::ReadFileToString(config_file_path, &contents)) {
return false;
}
- return Read(contents, /* fail_on_error */ false);
+ std::string error_msg;
+ if (!Read(contents, /* fail_on_error */ false, &error_msg)) {
+ LOG(ERROR) << error_msg;
+ return false;
+ }
+ if (!error_msg.empty()) {
+ LOG(WARNING) << error_msg;
+ }
+ return true;
}
-bool ConfigReader::Read(const std::string& content, bool fail_on_error) {
+bool ConfigReader::Read(const std::string& content, bool fail_on_error, std::string* error_msg) {
std::stringstream ss(content);
std::string line;
+
+ auto append_error = [error_msg](const std::string& tmp) {
+ if (!error_msg->empty()) {
+ error_msg->append("\n");
+ error_msg->append(tmp);
+ } else {
+ *error_msg = tmp;
+ }
+ };
+
for (unsigned linecount = 1;
std::getline(ss,line,'\n');
linecount += 1)
@@ -303,7 +334,7 @@
// look for X=Y assignment
auto efound = line.find('=');
if (efound == std::string::npos) {
- LOG(WARNING) << "line " << linecount << ": line malformed (no '=' found)";
+ append_error(StringPrintf("line %u: line malformed (no '=' found)", linecount));
if (fail_on_error) {
return false;
}
@@ -313,9 +344,13 @@
std::string key(line.substr(0, efound));
std::string value(line.substr(efound+1, std::string::npos));
- bool parse_success = parseLine(key.c_str(), value.c_str(), linecount);
- if (fail_on_error && !parse_success) {
- return false;
+ std::string local_error_msg;
+ bool parse_success = parseLine(key.c_str(), value.c_str(), linecount, &local_error_msg);
+ if (!parse_success) {
+ append_error(local_error_msg);
+ if (fail_on_error) {
+ return false;
+ }
}
}
diff --git a/perfprofd/configreader.h b/perfprofd/configreader.h
index 7b6bcf3..f849a37 100644
--- a/perfprofd/configreader.h
+++ b/perfprofd/configreader.h
@@ -44,7 +44,7 @@
// returns true for successful read, false if conf file cannot be opened.
bool readFile();
- bool Read(const std::string& data, bool fail_on_error);
+ bool Read(const std::string& data, bool fail_on_error, std::string* error_msg);
// set/get path to config file
static void setConfigFilePath(const char *path);
@@ -62,7 +62,10 @@
unsigned max_value);
void addStringEntry(const char *key, const char *default_value);
void addDefaultEntries();
- bool parseLine(const char *key, const char *value, unsigned linecount);
+ bool parseLine(const std::string& key,
+ const std::string& value,
+ unsigned linecount,
+ std::string* error_msg);
typedef struct { unsigned minv, maxv; } values;
std::map<std::string, values> u_info;
diff --git a/perfprofd/tests/perfprofd_test.cc b/perfprofd/tests/perfprofd_test.cc
index a96fd71..f17d1a6 100644
--- a/perfprofd/tests/perfprofd_test.cc
+++ b/perfprofd/tests/perfprofd_test.cc
@@ -625,8 +625,8 @@
// Verify log contents
const std::string expected = RAW_RESULT(
- W: line 6: malformed unsigned value (ignored)
- W: line 7: unknown option 'nonexistent_key' ignored
+ W: line 6: specified value 18446744073709551615 for 'collection_interval' outside permitted range [0 4294967295]
+ W: line 7: unknown option 'nonexistent_key'
W: line 8: line malformed (no '=' found)
);