From 1325ebfab2e490b7cb22900be2a859b133acb6eb Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Tue, 7 Jun 2016 13:03:10 -0700 Subject: [PATCH] Revert "logcat: expand -n, -r and -b" This reverts commit 33c262513f67d18a7a95d6cd0ec870f0d50dce86. Bug: 28120456 Bug: 22654233 Change-Id: I2a13f009d9e08dc2389b9872e45e95e0563af22f --- logcat/logcat.cpp | 113 ++++++----------------------------- logcat/tests/Android.mk | 2 +- logcat/tests/logcat_test.cpp | 9 --- 3 files changed, 20 insertions(+), 104 deletions(-) diff --git a/logcat/logcat.cpp b/logcat/logcat.cpp index 52f49cc3d..b8232cd40 100644 --- a/logcat/logcat.cpp +++ b/logcat/logcat.cpp @@ -27,7 +27,6 @@ #include #include -#include #include #include #include @@ -281,10 +280,10 @@ static void show_help(const char *cmd) fprintf(stderr, "options include:\n" " -s Set default filter to silent. Equivalent to filterspec '*:S'\n" " -f , --file= Log to file. Default is stdout\n" - " -r , --rotate-kbytes= Rotate log every kbytes. Requires -f\n" - " option. Permits property expansion.\n" - " -n , --rotate-count= Sets max number of rotated logs to\n" - " , default 4. Permits property expansion.\n" + " -r , --rotate-kbytes=\n" + " Rotate log every kbytes. Requires -f option\n" + " -n , --rotate-count=\n" + " Sets max number of rotated logs to , default 4\n" " -v , --format=\n" " Sets the log print format, where is:\n" " brief color epoch long monotonic printable process raw\n" @@ -318,7 +317,6 @@ static void show_help(const char *cmd) " 'system', 'radio', 'events', 'crash', 'default' or 'all'.\n" " Multiple -b parameters or comma separated list of buffers are\n" " allowed. Buffers interleaved. Default -b main,system,crash.\n" - " Permits property expansion.\n" " -B, --binary Output the log in binary.\n" " -S, --statistics Output statistics.\n" " -p, --prune Print prune white and ~black list. Service is specified as\n" @@ -336,11 +334,7 @@ static void show_help(const char *cmd) " comes first. Improves efficiency of polling by providing\n" " an about-to-wrap wakeup.\n"); - fprintf(stderr,"\nProperty expansion where available, may need to be single quoted to prevent\n" - "shell expansion:\n" - " ${key} - Expand string with property value associated with key\n" - " ${key:-default} - Expand, if property key value clear, use default\n" - "\nfilterspecs are a series of \n" + fprintf(stderr,"\nfilterspecs are a series of \n" " [:priority]\n\n" "where is a log component tag (or * for all) and priority is:\n" " V Verbose (default for )\n" @@ -538,49 +532,6 @@ static log_time lastLogTime(char *outputFileName) { return retval; } -// Expand multiple flat property references ${:-default} or ${tag}. -// -// ToDo: Do we permit nesting? -// ${persist.logcat.something:-${ro.logcat.something:-maybesomething}} -// For now this will result in a syntax error for caller and is acceptable. -// -std::string expand(const char *str) -{ - std::string retval(str); - - // Caller has no use for ${, } or :- as literals so no use for escape - // character. Result expectations are a number or a string, with validity - // checking for both in caller. Recursive expansion or other syntax errors - // will result in content caller can not obviously tolerate, error must - // report substring if applicable, expanded and original content (if - // different) so that it will be clear to user what they did wrong. - for (size_t pos; (pos = retval.find("${")) != std::string::npos; ) { - size_t epos = retval.find("}", pos + 2); - if (epos == std::string::npos) { - break; // Caller will error out, showing this unexpanded. - } - size_t def = retval.find(":-", pos + 2); - if (def >= epos) { - def = std::string::npos; - } - std::string default_value(""); - std::string key; - if (def == std::string::npos) { - key = retval.substr(pos + 2, epos - (pos + 2)); - } else { - key = retval.substr(pos + 2, def - (pos + 2)); - default_value = retval.substr(def + 2, epos - (def + 2)); - } - char value[PROPERTY_VALUE_MAX]; - property_get(key.c_str(), value, default_value.c_str()); - // Caller will error out, syntactically empty content at this point - // will not be tolerated as expected. - retval.replace(pos, epos - pos + 1, value); - } - - return retval; -} - } /* namespace android */ @@ -812,35 +763,23 @@ int main(int argc, char **argv) case 'b': { unsigned idMask = 0; - std::string expanded = expand(optarg); - std::istringstream copy(expanded); - std::string token; - // wish for strtok and ",:; \t\n\r\f" for hidden flexibility - while (std::getline(copy, token, ',')) { // settle for "," - if (token.compare("default") == 0) { + while ((optarg = strtok(optarg, ",:; \t\n\r\f")) != NULL) { + if (strcmp(optarg, "default") == 0) { idMask |= (1 << LOG_ID_MAIN) | (1 << LOG_ID_SYSTEM) | (1 << LOG_ID_CRASH); - } else if (token.compare("all") == 0) { + } else if (strcmp(optarg, "all") == 0) { idMask = (unsigned)-1; } else { - log_id_t log_id = android_name_to_log_id(token.c_str()); + log_id_t log_id = android_name_to_log_id(optarg); const char *name = android_log_id_to_name(log_id); - if (token.compare(name) != 0) { - bool strDifferent = expanded.compare(token); - if (expanded.compare(optarg)) { - expanded += " expanded from "; - expanded += optarg; - } - if (strDifferent) { - expanded = token + " within " + expanded; - } - logcat_panic(true, "unknown buffer -b %s\n", - expanded.c_str()); + if (strcmp(name, optarg) != 0) { + logcat_panic(true, "unknown buffer %s\n", optarg); } idMask |= (1 << log_id); } + optarg = NULL; } for (int i = LOG_ID_MIN; i < LOG_ID_MAX; ++i) { @@ -895,36 +834,22 @@ int main(int argc, char **argv) g_outputFileName = optarg; break; - case 'r': { - std::string expanded = expand(optarg); - if (!getSizeTArg(expanded.c_str(), &g_logRotateSizeKBytes, 1)) { - if (expanded.compare(optarg)) { - expanded += " expanded from "; - expanded += optarg; - } - logcat_panic(true, "Invalid parameter -r %s\n", - expanded.c_str()); + case 'r': + if (!getSizeTArg(optarg, &g_logRotateSizeKBytes, 1)) { + logcat_panic(true, "Invalid parameter %s to -r\n", optarg); } - } break; - case 'n': { - std::string expanded = expand(optarg); - if (!getSizeTArg(expanded.c_str(), &g_maxRotatedLogs, 1)) { - if (expanded.compare(optarg)) { - expanded += " expanded from "; - expanded += optarg; - } - logcat_panic(true, "Invalid parameter -n %s\n", - expanded.c_str()); + case 'n': + if (!getSizeTArg(optarg, &g_maxRotatedLogs, 1)) { + logcat_panic(true, "Invalid parameter %s to -n\n", optarg); } - } break; case 'v': err = setLogFormat (optarg); if (err < 0) { - logcat_panic(true, "Invalid parameter -v %s\n", optarg); + logcat_panic(true, "Invalid parameter %s to -v\n", optarg); } hasSetLogFormat |= err; break; diff --git a/logcat/tests/Android.mk b/logcat/tests/Android.mk index 3bf8a0bf9..a28664e30 100644 --- a/logcat/tests/Android.mk +++ b/logcat/tests/Android.mk @@ -56,6 +56,6 @@ include $(CLEAR_VARS) LOCAL_MODULE := $(test_module_prefix)unit-tests LOCAL_MODULE_TAGS := $(test_tags) LOCAL_CFLAGS += $(test_c_flags) -LOCAL_SHARED_LIBRARIES := liblog libcutils +LOCAL_SHARED_LIBRARIES := liblog LOCAL_SRC_FILES := $(test_src_files) include $(BUILD_NATIVE_TEST) diff --git a/logcat/tests/logcat_test.cpp b/logcat/tests/logcat_test.cpp index 920d50447..d46e9ff41 100644 --- a/logcat/tests/logcat_test.cpp +++ b/logcat/tests/logcat_test.cpp @@ -25,7 +25,6 @@ #include -#include #include #include #include @@ -426,14 +425,6 @@ TEST(logcat, multiple_buffer) { "logcat -v brief -b radio,events,system,main -g 2>/dev/null")); } -// duplicate test for get_size, but use test.logcat.buffer property -TEST(logcat, property_expand) { - property_set("test.logcat.buffer", "radio,events"); - EXPECT_EQ(4, get_groups( - "logcat -v brief -b 'system,${test.logcat.buffer:-bogo},main' -g 2>/dev/null")); - property_set("test.logcat.buffer", ""); -} - TEST(logcat, bad_buffer) { ASSERT_EQ(0, get_groups( "logcat -v brief -b radio,events,bogo,system,main -g 2>/dev/null"));