From df50f9440dc4a56a67d9150d24069f379dde74e5 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 16 Feb 2015 13:13:39 +0000 Subject: [PATCH] Handle trailing spaces on "settings set" command more correctly Summary: Currently we have some settings which treat "\ " on settings set commands specially. E.g., it is a valid way of specifying an argument of " " to a target. However, this fails if "\ " is the last argument as CommandObjectSettingsSet strips trailing whitespace. This resulted in a surprising argument of "\" to the target. This patch disables the training whitespace removal at a global level. Instead, for each argument type we locally determine whether whitespace stripping makes sense. Currently, I strip whitespace for all simple object type except of regex and format-string, with the rationale that these two object types do their own complex parsing and we want to interfere with them as least as possible. Specifically, stripping the whitespace of a regex "\ " will result in a (surprising?) error "trailing backslash". Furthermore, the default value of dissasembly-format setting already contains a trailing space and there is no way for the user to type this in manually if we strip whitespace. Reviewers: clayborg, zturner Subscribers: lldb-commits Differential Revision: http://reviews.llvm.org/D7592 llvm-svn: 229382 --- lldb/source/Commands/CommandObjectSettings.cpp | 2 +- lldb/source/Interpreter/Args.cpp | 36 ++++++++++------------ lldb/source/Interpreter/OptionValueArch.cpp | 4 ++- lldb/source/Interpreter/OptionValueEnumeration.cpp | 2 +- lldb/source/Interpreter/OptionValueSInt64.cpp | 3 +- lldb/source/Interpreter/OptionValueString.cpp | 13 ++++---- lldb/source/Interpreter/OptionValueUInt64.cpp | 3 +- lldb/test/settings/TestSettings.py | 21 +++++++++++++ 8 files changed, 54 insertions(+), 30 deletions(-) diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp index ed677af..b7cbb7d 100644 --- a/lldb/source/Commands/CommandObjectSettings.cpp +++ b/lldb/source/Commands/CommandObjectSettings.cpp @@ -243,7 +243,7 @@ protected: // Split the raw command into var_name and value pair. llvm::StringRef raw_str(command); std::string var_value_string = raw_str.split(var_name).second.str(); - const char *var_value_cstr = Args::StripSpaces(var_value_string, true, true, false); + const char *var_value_cstr = Args::StripSpaces(var_value_string, true, false, false); Error error; if (m_options.m_global) diff --git a/lldb/source/Interpreter/Args.cpp b/lldb/source/Interpreter/Args.cpp index 4f0219f..50d3fff 100644 --- a/lldb/source/Interpreter/Args.cpp +++ b/lldb/source/Interpreter/Args.cpp @@ -870,26 +870,24 @@ Args::StripSpaces (std::string &s, bool leading, bool trailing, bool return_null bool Args::StringToBoolean (const char *s, bool fail_value, bool *success_ptr) { - if (s && s[0]) + llvm::StringRef ref = llvm::StringRef(s).trim(); + if (ref.equals_lower("false") || + ref.equals_lower("off") || + ref.equals_lower("no") || + ref.equals_lower("0")) { - if (::strcasecmp (s, "false") == 0 || - ::strcasecmp (s, "off") == 0 || - ::strcasecmp (s, "no") == 0 || - ::strcmp (s, "0") == 0) - { - if (success_ptr) - *success_ptr = true; - return false; - } - else - if (::strcasecmp (s, "true") == 0 || - ::strcasecmp (s, "on") == 0 || - ::strcasecmp (s, "yes") == 0 || - ::strcmp (s, "1") == 0) - { - if (success_ptr) *success_ptr = true; - return true; - } + if (success_ptr) + *success_ptr = true; + return false; + } + else + if (ref.equals_lower("true") || + ref.equals_lower("on") || + ref.equals_lower("yes") || + ref.equals_lower("1")) + { + if (success_ptr) *success_ptr = true; + return true; } if (success_ptr) *success_ptr = false; return fail_value; diff --git a/lldb/source/Interpreter/OptionValueArch.cpp b/lldb/source/Interpreter/OptionValueArch.cpp index 7df1492..eae903c 100644 --- a/lldb/source/Interpreter/OptionValueArch.cpp +++ b/lldb/source/Interpreter/OptionValueArch.cpp @@ -55,8 +55,10 @@ OptionValueArch::SetValueFromCString (const char *value_cstr, VarSetOperationTyp case eVarSetOperationReplace: case eVarSetOperationAssign: - if (value_cstr && value_cstr[0]) + if (value_cstr) { + std::string value = llvm::StringRef(value_cstr).trim().str(); + value_cstr = value.c_str(); if (m_current_value.SetTriple (value_cstr)) { m_value_was_set = true; diff --git a/lldb/source/Interpreter/OptionValueEnumeration.cpp b/lldb/source/Interpreter/OptionValueEnumeration.cpp index dbaeb18..b5cdd81 100644 --- a/lldb/source/Interpreter/OptionValueEnumeration.cpp +++ b/lldb/source/Interpreter/OptionValueEnumeration.cpp @@ -69,7 +69,7 @@ OptionValueEnumeration::SetValueFromCString (const char *value, VarSetOperationT case eVarSetOperationAssign: if (value && value[0]) { - ConstString const_enumerator_name(value); + ConstString const_enumerator_name(llvm::StringRef(value).trim()); const EnumerationMapEntry *enumerator_entry = m_enumerations.FindFirstValueForName (const_enumerator_name.GetCString()); if (enumerator_entry) { diff --git a/lldb/source/Interpreter/OptionValueSInt64.cpp b/lldb/source/Interpreter/OptionValueSInt64.cpp index c691729..d62353d 100644 --- a/lldb/source/Interpreter/OptionValueSInt64.cpp +++ b/lldb/source/Interpreter/OptionValueSInt64.cpp @@ -51,7 +51,8 @@ OptionValueSInt64::SetValueFromCString (const char *value_cstr, VarSetOperationT case eVarSetOperationAssign: { bool success = false; - int64_t value = StringConvert::ToSInt64 (value_cstr, 0, 0, &success); + std::string value_str = llvm::StringRef(value_cstr).trim().str(); + int64_t value = StringConvert::ToSInt64 (value_str.c_str(), 0, 0, &success); if (success) { if (value >= m_min_value && value <= m_max_value) diff --git a/lldb/source/Interpreter/OptionValueString.cpp b/lldb/source/Interpreter/OptionValueString.cpp index a1b80d8..9012ab1 100644 --- a/lldb/source/Interpreter/OptionValueString.cpp +++ b/lldb/source/Interpreter/OptionValueString.cpp @@ -57,24 +57,25 @@ OptionValueString::SetValueFromCString (const char *value_cstr, Error error; std::string value_str_no_quotes; - if (value_cstr) + llvm::StringRef trimmed = value_cstr ? llvm::StringRef(value_cstr).trim() : llvm::StringRef(); + if (trimmed.size() > 0) { - switch (value_cstr[0]) + switch (trimmed.front()) { case '"': case '\'': { - size_t len = strlen(value_cstr); - if (len <= 1 || value_cstr[len-1] != value_cstr[0]) + if (trimmed.size() <= 1 || trimmed.back() != trimmed.front()) { error.SetErrorString("mismatched quotes"); return error; } - value_str_no_quotes.assign (value_cstr + 1, len - 2); - value_cstr = value_str_no_quotes.c_str(); + trimmed = trimmed.drop_front().drop_back().str(); } break; } + value_str_no_quotes = trimmed.str(); + value_cstr = value_str_no_quotes.c_str(); } switch (op) diff --git a/lldb/source/Interpreter/OptionValueUInt64.cpp b/lldb/source/Interpreter/OptionValueUInt64.cpp index 48de433..dc06cfe 100644 --- a/lldb/source/Interpreter/OptionValueUInt64.cpp +++ b/lldb/source/Interpreter/OptionValueUInt64.cpp @@ -58,7 +58,8 @@ OptionValueUInt64::SetValueFromCString (const char *value_cstr, VarSetOperationT case eVarSetOperationAssign: { bool success = false; - uint64_t value = StringConvert::ToUInt64 (value_cstr, 0, 0, &success); + std::string value_str = llvm::StringRef(value_cstr).trim().str(); + uint64_t value = StringConvert::ToUInt64 (value_str.c_str(), 0, 0, &success); if (success) { m_value_was_set = true; diff --git a/lldb/test/settings/TestSettings.py b/lldb/test/settings/TestSettings.py index c0670b7..2cb57da 100644 --- a/lldb/test/settings/TestSettings.py +++ b/lldb/test/settings/TestSettings.py @@ -396,6 +396,13 @@ class SettingsCommandTestCase(TestBase): '[0]: "3"', '[1]: "4"', '[2]: "5"' ]) + self.runCmd ("settings set target.run-args 1 2 3") # Set to known value + self.runCmd ("settings set target.run-args 3 \ \ ") # Set to new value with trailing whitespaces + self.expect ("settings show target.run-args", SETTING_MSG("target.run-args"), + substrs = [ 'target.run-args (arguments) =', + '[0]: "3"', + '[1]: " "', + '[2]: " "' ]) self.runCmd("settings clear target.run-args", check=False) # dictionaries self.runCmd ("settings clear target.env-vars") # Set to known value @@ -405,6 +412,20 @@ class SettingsCommandTestCase(TestBase): 'A=B', 'C=D']) self.runCmd("settings clear target.env-vars", check=False) + # regex + self.runCmd ("settings clear target.process.thread.step-avoid-regexp") # Set to known value + self.runCmd ("settings set target.process.thread.step-avoid-regexp foo\\ ") # Set to new value with trailing whitespaces + self.expect ("settings show target.process.thread.step-avoid-regexp", + SETTING_MSG("target.process.thread.step-avoid-regexp"), + substrs = [ 'target.process.thread.step-avoid-regexp (regex) = foo\\ ']) + self.runCmd("settings clear target.process.thread.step-avoid-regexp", check=False) + # format-string + self.runCmd ("settings clear disassembly-format") # Set to known value + self.runCmd ("settings set disassembly-format foo ") # Set to new value with trailing whitespaces + self.expect ("settings show disassembly-format", + SETTING_MSG("disassembly-format"), + substrs = [ 'disassembly-format (format-string) = "foo "']) + self.runCmd("settings clear disassembly-format", check=False) def test_all_settings_exist (self): self.expect ("settings show", -- 2.7.4