From 9d14084b45320a1b4ff75eec7ede88b15ac3389c Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Tue, 11 Dec 2012 22:42:19 +0000 Subject: [PATCH] Adding a validation callback mechanism to OptionValueString (such a feature might theoretically be added to the general OptionValue base class should the need arise) Using this mechanism, making sure that the options to pass a summary string or a named summary to frame variable do not have invalid values llvm-svn: 169927 --- lldb/include/lldb/Interpreter/OptionValueString.h | 90 ++++++++++++++++------ lldb/source/Interpreter/OptionGroupVariable.cpp | 23 +++++- lldb/source/Interpreter/OptionValueString.cpp | 62 ++++++++++++++- .../TestDataFormatterNamedSummaries.py | 7 +- 4 files changed, 151 insertions(+), 31 deletions(-) diff --git a/lldb/include/lldb/Interpreter/OptionValueString.h b/lldb/include/lldb/Interpreter/OptionValueString.h index 0ecb406..a82e140 100644 --- a/lldb/include/lldb/Interpreter/OptionValueString.h +++ b/lldb/include/lldb/Interpreter/OptionValueString.h @@ -24,6 +24,10 @@ namespace lldb_private { class OptionValueString : public OptionValue { public: + + typedef Error (*ValidatorCallback) (const char* string, + void* baton); + enum Options { eOptionEncodeCharacterEscapeSequences = (1u << 0) @@ -33,7 +37,20 @@ public: OptionValue(), m_current_value (), m_default_value (), - m_options() + m_options(), + m_validator(), + m_validator_baton() + { + } + + OptionValueString (ValidatorCallback validator, + void* baton = NULL) : + OptionValue(), + m_current_value (), + m_default_value (), + m_options(), + m_validator(validator), + m_validator_baton(baton) { } @@ -41,7 +58,9 @@ public: OptionValue(), m_current_value (), m_default_value (), - m_options() + m_options(), + m_validator(), + m_validator_baton() { if (value && value[0]) { @@ -55,7 +74,9 @@ public: OptionValue(), m_current_value (), m_default_value (), - m_options() + m_options(), + m_validator(), + m_validator_baton() { if (current_value && current_value[0]) m_current_value.assign (current_value); @@ -63,7 +84,41 @@ public: m_default_value.assign (default_value); } - virtual + OptionValueString (const char *value, + ValidatorCallback validator, + void* baton = NULL) : + OptionValue(), + m_current_value (), + m_default_value (), + m_options(), + m_validator(validator), + m_validator_baton(baton) + { + if (value && value[0]) + { + m_current_value.assign (value); + m_default_value.assign (value); + } + } + + OptionValueString (const char *current_value, + const char *default_value, + ValidatorCallback validator, + void* baton = NULL) : + OptionValue(), + m_current_value (), + m_default_value (), + m_options(), + m_validator(validator), + m_validator_baton(baton) + { + if (current_value && current_value[0]) + m_current_value.assign (current_value); + if (default_value && default_value[0]) + m_default_value.assign (default_value); + } + + virtual ~OptionValueString() { } @@ -115,10 +170,7 @@ public: const char * operator = (const char *value) { - if (value && value[0]) - m_current_value.assign (value); - else - m_current_value.clear(); + SetCurrentValue(value); return m_current_value.c_str(); } @@ -134,21 +186,11 @@ public: return m_default_value.c_str(); } - void - SetCurrentValue (const char *value) - { - if (value && value[0]) - m_current_value.assign (value); - else - m_current_value.clear(); - } - - void - AppendToCurrentValue (const char *value) - { - if (value && value[0]) - m_current_value.append (value); - } + Error + SetCurrentValue (const char *value); + + Error + AppendToCurrentValue (const char *value); void SetDefaultValue (const char *value) @@ -176,6 +218,8 @@ protected: std::string m_current_value; std::string m_default_value; Flags m_options; + ValidatorCallback m_validator; + void* m_validator_baton; }; } // namespace lldb_private diff --git a/lldb/source/Interpreter/OptionGroupVariable.cpp b/lldb/source/Interpreter/OptionGroupVariable.cpp index 5f1c271..e98ff56 100644 --- a/lldb/source/Interpreter/OptionGroupVariable.cpp +++ b/lldb/source/Interpreter/OptionGroupVariable.cpp @@ -15,6 +15,8 @@ // C++ Includes // Other libraries and framework includes // Project includes +#include "lldb/Core/DataVisualization.h" +#include "lldb/Core/Error.h" #include "lldb/Target/Target.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Utility/Utils.h" @@ -39,7 +41,22 @@ g_option_table[] = OptionGroupVariable::OptionGroupVariable (bool show_frame_options) : OptionGroup(), - include_frame_options (show_frame_options) + include_frame_options (show_frame_options), + summary([] (const char* str,void*)->Error + { + if (!str || !str[0]) + return Error("must specify a valid named summary"); + TypeSummaryImplSP summary_sp; + if (DataVisualization::NamedSummaryFormats::GetSummaryFormat(ConstString(str), summary_sp) == false) + return Error("must specify a valid named summary"); + return Error(); + }), + summary_string([] (const char* str, void*)->Error + { + if (!str || !str[0]) + return Error("must specify a non-empty summary string"); + return Error(); + }) { } @@ -67,10 +84,10 @@ OptionGroupVariable::SetOptionValue (CommandInterpreter &interpreter, show_scope = true; break; case 'y': - summary.SetCurrentValue(option_arg); + error = summary.SetCurrentValue(option_arg); break; case 'z': - summary_string.SetCurrentValue(option_arg); + error = summary_string.SetCurrentValue(option_arg); break; default: error.SetErrorStringWithFormat("unrecognized short option '%c'", short_option); diff --git a/lldb/source/Interpreter/OptionValueString.cpp b/lldb/source/Interpreter/OptionValueString.cpp index 696e62b7..91a9803 100644 --- a/lldb/source/Interpreter/OptionValueString.cpp +++ b/lldb/source/Interpreter/OptionValueString.cpp @@ -61,20 +61,36 @@ OptionValueString::SetValueFromCString (const char *value_cstr, case eVarSetOperationInsertBefore: case eVarSetOperationInsertAfter: case eVarSetOperationRemove: + if (m_validator) + { + error = m_validator(value_cstr,m_validator_baton); + if (error.Fail()) + return error; + } error = OptionValue::SetValueFromCString (value_cstr, op); break; case eVarSetOperationAppend: + { + std::string new_value(m_current_value); if (value_cstr && value_cstr[0]) { if (m_options.Test (eOptionEncodeCharacterEscapeSequences)) { std::string str; Args::EncodeEscapeSequences (value_cstr, str); - m_current_value += str; + new_value.append(str); } else - m_current_value += value_cstr; + new_value.append(value_cstr); + } + if (m_validator) + { + error = m_validator(new_value.c_str(),m_validator_baton); + if (error.Fail()) + return error; + } + m_current_value.assign(new_value); } break; @@ -84,6 +100,12 @@ OptionValueString::SetValueFromCString (const char *value_cstr, case eVarSetOperationReplace: case eVarSetOperationAssign: + if (m_validator) + { + error = m_validator(value_cstr,m_validator_baton); + if (error.Fail()) + return error; + } m_value_was_set = true; if (m_options.Test (eOptionEncodeCharacterEscapeSequences)) { @@ -104,3 +126,39 @@ OptionValueString::DeepCopy () const { return OptionValueSP(new OptionValueString(*this)); } + +Error +OptionValueString::SetCurrentValue (const char *value) +{ + if (m_validator) + { + Error error(m_validator(value,m_validator_baton)); + if (error.Fail()) + return error; + } + if (value && value[0]) + m_current_value.assign (value); + else + m_current_value.clear(); + return Error(); +} + +Error +OptionValueString::AppendToCurrentValue (const char *value) +{ + if (value && value[0]) + { + if (m_validator) + { + std::string new_value(m_current_value); + new_value.append(value); + Error error(m_validator(value,m_validator_baton)); + if (error.Fail()) + return error; + m_current_value.assign(new_value); + } + else + m_current_value.append (value); + } + return Error(); +} diff --git a/lldb/test/functionalities/data-formatter/data-formatter-named-summaries/TestDataFormatterNamedSummaries.py b/lldb/test/functionalities/data-formatter/data-formatter-named-summaries/TestDataFormatterNamedSummaries.py index de4da12..1507ece 100644 --- a/lldb/test/functionalities/data-formatter/data-formatter-named-summaries/TestDataFormatterNamedSummaries.py +++ b/lldb/test/functionalities/data-formatter/data-formatter-named-summaries/TestDataFormatterNamedSummaries.py @@ -94,9 +94,10 @@ class NamedSummariesDataFormatterTestCase(TestBase): substrs = ['Second: x=65', 'y=0x']) - self.expect("frame variable second --summary NoSuchSummary", - substrs = ['Second: x=65', - 'y=0x']) + # decided that invalid summaries will raise an error + # instead of just defaulting to the base summary + self.expect("frame variable second --summary NoSuchSummary",error=True, + substrs = ['must specify a valid named summary']) self.runCmd("thread step-over") -- 2.7.4