zturner created this revision. zturner added a reviewer: clayborg. zturner added subscribers: lldb-commits, LLDB.
The `Args` class was trying to maintain two parallel argument vectors. One that owned the memory and contained `std::strings`, and one that contained `const char *`'s that could be used similar to how `argv` is used. But dealing with pointers to pointers is unsafe and should be avoided wherever possible. Furthermore, there doesn't seem to be much point in maintaining a second copy of the array, as it only serves to complicate the parsing logic since the two have to be kept in sync with each other, and really, the master copy has everything you need, including the null terminated strings. So I removed `m_argv` entirely. The `Args` class will also no longer vend a `const char**`, and instead you can pass it a `std::vector<const char*>&` and it will fill it with null terminated argument strings. This can be used exactly like a `const char**` by writing `&args[0]`, but now you have to explicitly opt into this, and by default you just get a nice safe container that can be used for example in ranged based fors. And the logic is simplified as well since the two arrays don't have to be kept in sync with each other. All tests pass on Windows. https://reviews.llvm.org/D24952 Files: examples/plugins/commands/fooplugin.cpp include/lldb/API/SBCommandInterpreter.h include/lldb/Host/FileSpec.h include/lldb/Host/OptionParser.h include/lldb/Interpreter/Args.h source/API/SBCommandInterpreter.cpp source/API/SBLaunchInfo.cpp source/API/SBProcess.cpp source/API/SBTarget.cpp source/Commands/CommandObjectBreakpoint.cpp source/Commands/CommandObjectLog.cpp source/Host/common/FileSpec.cpp source/Host/common/OptionParser.cpp source/Interpreter/Args.cpp source/Interpreter/CommandInterpreter.cpp source/Interpreter/CommandObject.cpp source/Interpreter/OptionValueArgs.cpp source/Interpreter/OptionValueArray.cpp source/Plugins/Platform/MacOSX/PlatformDarwin.cpp source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp source/Target/ProcessInfo.cpp source/Target/ProcessLaunchInfo.cpp
Index: source/Target/ProcessLaunchInfo.cpp =================================================================== --- source/Target/ProcessLaunchInfo.cpp +++ source/Target/ProcessLaunchInfo.cpp @@ -339,8 +339,9 @@ if (m_shell) { std::string shell_executable = m_shell.GetPath(); - const char **argv = GetArguments().GetConstArgumentVector(); - if (argv == nullptr || argv[0] == nullptr) + std::vector<const char *> argv; + GetArguments().GetArgumentVector(argv); + if (!argv[0]) return false; Args shell_arguments; std::string safe_arg; Index: source/Target/ProcessInfo.cpp =================================================================== --- source/Target/ProcessInfo.cpp +++ source/Target/ProcessInfo.cpp @@ -91,7 +91,7 @@ void ProcessInfo::SetArguments(char const **argv, bool first_arg_is_executable) { - m_arguments.SetArguments(argv); + m_arguments.SetArguments(Args::ArgvToArrayRef(argv)); // Is the first argument the executable? if (first_arg_is_executable) { Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -421,15 +421,14 @@ } // Send the environment and the program + arguments after we connect - const char **envp = - launch_info.GetEnvironmentEntries().GetConstArgumentVector(); - - if (envp) { - const char *env_entry; - for (int i = 0; (env_entry = envp[i]); ++i) { - if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0) - break; - } + std::vector<const char *> argv; + launch_info.GetEnvironmentEntries().GetArgumentVector(argv); + const char **envp = &argv[0]; + + const char *env_entry; + for (int i = 0; (env_entry = envp[i]); ++i) { + if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0) + break; } ArchSpec arch_spec = launch_info.GetArchitecture(); Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp =================================================================== --- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1402,13 +1402,12 @@ // /bin/sh re-exec's itself as /bin/bash requiring another resume. // But it only does this if the COMMAND_MODE environment variable // is set to "legacy". - const char **envp = - launch_info.GetEnvironmentEntries().GetConstArgumentVector(); - if (envp != NULL) { - for (int i = 0; envp[i] != NULL; i++) { - if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0) - return 2; - } + std::vector<const char *> argv; + launch_info.GetEnvironmentEntries().GetArgumentVector(argv); + const char **envp = &argv[0]; + for (int i = 0; envp[i] != NULL; i++) { + if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0) + return 2; } return 1; } else if (strcmp(shell_name, "csh") == 0 || Index: source/Interpreter/OptionValueArray.cpp =================================================================== --- source/Interpreter/OptionValueArray.cpp +++ source/Interpreter/OptionValueArray.cpp @@ -148,7 +148,7 @@ if (argv.empty()) args.Clear(); else - args.SetArguments(argv.size(), &argv[0]); + args.SetArguments(argv); return args.GetArgumentCount(); } Index: source/Interpreter/OptionValueArgs.cpp =================================================================== --- source/Interpreter/OptionValueArgs.cpp +++ source/Interpreter/OptionValueArgs.cpp @@ -30,6 +30,6 @@ if (argv.empty()) args.Clear(); else - args.SetArguments(argv.size(), &argv[0]); + args.SetArguments(argv); return args.GetArgumentCount(); } Index: source/Interpreter/CommandObject.cpp =================================================================== --- source/Interpreter/CommandObject.cpp +++ source/Interpreter/CommandObject.cpp @@ -116,7 +116,6 @@ // ParseOptions calls getopt_long_only, which always skips the zero'th item // in the array and starts at position 1, // so we need to push a dummy value into position zero. - args.Unshift(llvm::StringRef("dummy_string")); const bool require_validation = true; error = args.ParseOptions(*options, &exe_ctx, GetCommandInterpreter().GetPlatform(true), @@ -992,8 +991,10 @@ if (HasOverrideCallback()) { Args full_args(GetCommandName()); full_args.AppendArguments(cmd_args); - handled = - InvokeOverrideCallback(full_args.GetConstArgumentVector(), result); + + std::vector<const char *> argv; + full_args.GetArgumentVector(argv); + handled = InvokeOverrideCallback(&argv[0], result); } if (!handled) { for (size_t i = 0; i < cmd_args.GetArgumentCount(); ++i) { Index: source/Interpreter/CommandInterpreter.cpp =================================================================== --- source/Interpreter/CommandInterpreter.cpp +++ source/Interpreter/CommandInterpreter.cpp @@ -2047,8 +2047,7 @@ } cmd_args.Clear(); - cmd_args.SetArguments(new_args.GetArgumentCount(), - new_args.GetConstArgumentVector()); + cmd_args.SetArguments(new_args); } else { result.SetStatus(eReturnStatusSuccessFinishNoResult); // This alias was not created with any options; nothing further needs to be @@ -2058,8 +2057,7 @@ // input string. if (wants_raw_input) { cmd_args.Clear(); - cmd_args.SetArguments(new_args.GetArgumentCount(), - new_args.GetConstArgumentVector()); + cmd_args.SetArguments(new_args); } return; } Index: source/Interpreter/Args.cpp =================================================================== --- source/Interpreter/Args.cpp +++ source/Interpreter/Args.cpp @@ -25,15 +25,16 @@ #include "lldb/Target/StackFrame.h" #include "lldb/Target/Target.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringSwitch.h" using namespace lldb; using namespace lldb_private; //---------------------------------------------------------------------- // Args constructor //---------------------------------------------------------------------- -Args::Args(llvm::StringRef command) : m_args(), m_argv(), m_args_quote_char() { +Args::Args(llvm::StringRef command) : m_args(), m_args_quote_char() { SetCommandString(command); } @@ -45,9 +46,7 @@ // own m_argv appropriately. //---------------------------------------------------------------------- Args::Args(const Args &rhs) - : m_args(rhs.m_args), m_argv(), m_args_quote_char(rhs.m_args_quote_char) { - UpdateArgvFromArgs(); -} + : m_args(rhs.m_args), m_args_quote_char(rhs.m_args_quote_char) {} //---------------------------------------------------------------------- // We have to be very careful on the copy constructor of this class @@ -61,7 +60,6 @@ if (this != &rhs) { m_args = rhs.m_args; m_args_quote_char = rhs.m_args_quote_char; - UpdateArgvFromArgs(); } return *this; } @@ -75,42 +73,32 @@ if (!label_name) return; - const size_t argc = m_argv.size(); - for (size_t i = 0; i < argc; ++i) { + for (int i = 0; i < m_args.size(); ++i) { s.Indent(); - const char *arg_cstr = m_argv[i]; - if (arg_cstr) - s.Printf("%s[%zi]=\"%s\"\n", label_name, i, arg_cstr); - else - s.Printf("%s[%zi]=NULL\n", label_name, i); + s.Printf("%s[%zi]=\"%s\"\n", label_name, i, m_args[i].c_str()); } s.EOL(); } bool Args::GetCommandString(std::string &command) const { - command.clear(); - const size_t argc = GetArgumentCount(); - for (size_t i = 0; i < argc; ++i) { - if (i > 0) - command += ' '; - command += m_argv[i]; - } - return argc > 0; + command = llvm::join(m_args.begin(), m_args.end(), " "); + + return !m_args.empty(); } bool Args::GetQuotedCommandString(std::string &command) const { command.clear(); const size_t argc = GetArgumentCount(); for (size_t i = 0; i < argc; ++i) { if (i > 0) - command.append(1, ' '); + command += ' '; char quote_char = GetArgumentQuoteCharAtIndex(i); if (quote_char) { - command.append(1, quote_char); - command.append(m_argv[i]); - command.append(1, quote_char); + command += quote_char; + command += m_args[i]; + command += quote_char; } else - command.append(m_argv[i]); + command += m_args[i]; } return argc > 0; } @@ -253,74 +241,51 @@ void Args::SetCommandString(llvm::StringRef command) { m_args.clear(); - m_argv.clear(); m_args_quote_char.clear(); static const char *k_space_separators = " \t"; command = command.ltrim(k_space_separators); while (!command.empty()) { command = ParseSingleArgument(command); command = command.ltrim(k_space_separators); } - - UpdateArgvFromArgs(); } -void Args::UpdateArgsAfterOptionParsing() { - // Now m_argv might be out of date with m_args, so we need to fix that - arg_cstr_collection::const_iterator argv_pos, argv_end = m_argv.end(); - arg_sstr_collection::iterator args_pos; - arg_quote_char_collection::iterator quotes_pos; - - for (argv_pos = m_argv.begin(), args_pos = m_args.begin(), - quotes_pos = m_args_quote_char.begin(); - argv_pos != argv_end && args_pos != m_args.end(); ++argv_pos) { - const char *argv_cstr = *argv_pos; - if (argv_cstr == nullptr) - break; - - while (args_pos != m_args.end()) { - const char *args_cstr = args_pos->c_str(); - if (args_cstr == argv_cstr) { - // We found the argument that matches the C string in the - // vector, so we can now look for the next one - ++args_pos; - ++quotes_pos; - break; - } else { - quotes_pos = m_args_quote_char.erase(quotes_pos); - args_pos = m_args.erase(args_pos); - } - } +void Args::UpdateArgsAfterOptionParsing(llvm::ArrayRef<const char *> new_argv) { + // By now many of the arguments will have been removed since they were + // consumed. + // Some options will remain and be potentially in a different order than they + // started in. For the options that remain, map them back to their original + // location so we can determine their quote char. Then move their quote char + // to the correct new location. + for (int i = 0; i < new_argv.size(); ++i) { + auto iter = std::find(m_args.begin(), m_args.end(), new_argv[i]); + lldbassert(iter != m_args.end()); + size_t pos = std::distance(m_args.begin(), iter); + std::swap(m_args_quote_char[pos], m_args_quote_char[i]); } - - if (args_pos != m_args.end()) - m_args.erase(args_pos, m_args.end()); - - if (quotes_pos != m_args_quote_char.end()) - m_args_quote_char.erase(quotes_pos, m_args_quote_char.end()); -} - -void Args::UpdateArgvFromArgs() { - m_argv.clear(); - arg_sstr_collection::const_iterator pos, end = m_args.end(); - for (pos = m_args.begin(); pos != end; ++pos) - m_argv.push_back(pos->c_str()); - m_argv.push_back(nullptr); - // Make sure we have enough arg quote chars in the array - if (m_args_quote_char.size() < m_args.size()) - m_args_quote_char.resize(m_argv.size()); + m_args_quote_char.resize(new_argv.size()); + + // Since new_argv is a collection of *references* to strings from m_args, we + // can't + // simply assign it directly, as saying m_args[x] = new_argv[y] would + // invalidate the + // existing value of m_args[x], which is itself another point somewhere in + // new_argv. + // So make a copy and then swap it in. + std::vector<std::string> new_args(new_argv.begin(), new_argv.end()); + std::swap(m_args, new_args); } size_t Args::GetArgumentCount() const { - if (m_argv.empty()) + if (m_args.empty()) return 0; - return m_argv.size() - 1; + return m_args.size(); } const char *Args::GetArgumentAtIndex(size_t idx) const { - if (idx < m_argv.size()) - return m_argv[idx]; + if (idx < m_args.size()) + return m_args[idx].c_str(); return nullptr; } @@ -330,152 +295,100 @@ return '\0'; } -char **Args::GetArgumentVector() { - if (!m_argv.empty()) - return const_cast<char **>(&m_argv[0]); - return nullptr; -} +void Args::GetArgumentVector(std::vector<const char *> &args) const { + args.clear(); + for (auto &arg : m_args) + args.push_back(arg.c_str()); -const char **Args::GetConstArgumentVector() const { - if (!m_argv.empty()) - return const_cast<const char **>(&m_argv[0]); - return nullptr; + // This is kind of gross, but it ensures that if someone passes args.data() to + // a function which expects an array terminated with a null entry, it will + // work. + args.push_back(nullptr); + args.pop_back(); } void Args::Shift() { - // Don't pop the last NULL terminator from the argv array - if (m_argv.size() > 1) { - m_argv.erase(m_argv.begin()); - m_args.pop_front(); - if (!m_args_quote_char.empty()) - m_args_quote_char.erase(m_args_quote_char.begin()); - } + if (m_args.empty()) + return; + + m_args.erase(m_args.begin()); + if (!m_args_quote_char.empty()) + m_args_quote_char.erase(m_args_quote_char.begin()); } llvm::StringRef Args::Unshift(llvm::StringRef arg_str, char quote_char) { - m_args.push_front(arg_str); - m_argv.insert(m_argv.begin(), m_args.front().c_str()); + m_args.insert(m_args.begin(), arg_str); m_args_quote_char.insert(m_args_quote_char.begin(), quote_char); - return llvm::StringRef::withNullAsEmpty(GetArgumentAtIndex(0)); + return m_args[0]; } void Args::AppendArguments(const Args &rhs) { const size_t rhs_argc = rhs.GetArgumentCount(); for (size_t i = 0; i < rhs_argc; ++i) - AppendArgument(llvm::StringRef(rhs.GetArgumentAtIndex(i)), + AppendArgument(rhs.GetArgumentAtIndex(i), rhs.GetArgumentQuoteCharAtIndex(i)); } -void Args::AppendArguments(const char **argv) { - if (argv) { - for (uint32_t i = 0; argv[i]; ++i) - AppendArgument(llvm::StringRef::withNullAsEmpty(argv[i])); - } +void Args::AppendArguments(llvm::ArrayRef<const char *> args) { + for (uint32_t i = 0; args[i]; ++i) + AppendArgument(args[i]); } llvm::StringRef Args::AppendArgument(llvm::StringRef arg_str, char quote_char) { return InsertArgumentAtIndex(GetArgumentCount(), arg_str, quote_char); } llvm::StringRef Args::InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str, char quote_char) { - // Since we are using a std::list to hold onto the copied C string and - // we don't have direct access to the elements, we have to iterate to - // find the value. - arg_sstr_collection::iterator pos, end = m_args.end(); - size_t i = idx; - for (pos = m_args.begin(); i > 0 && pos != end; ++pos) - --i; - - pos = m_args.insert(pos, arg_str); + m_args.insert(m_args.begin() + idx, arg_str); if (idx >= m_args_quote_char.size()) { m_args_quote_char.resize(idx + 1); m_args_quote_char[idx] = quote_char; } else m_args_quote_char.insert(m_args_quote_char.begin() + idx, quote_char); - UpdateArgvFromArgs(); return GetArgumentAtIndex(idx); } llvm::StringRef Args::ReplaceArgumentAtIndex(size_t idx, llvm::StringRef arg_str, char quote_char) { - // Since we are using a std::list to hold onto the copied C string and - // we don't have direct access to the elements, we have to iterate to - // find the value. - arg_sstr_collection::iterator pos, end = m_args.end(); - size_t i = idx; - for (pos = m_args.begin(); i > 0 && pos != end; ++pos) - --i; - - if (pos != end) { - pos->assign(arg_str); - assert(idx < m_argv.size() - 1); - m_argv[idx] = pos->c_str(); - if (idx >= m_args_quote_char.size()) - m_args_quote_char.resize(idx + 1); - m_args_quote_char[idx] = quote_char; - return GetArgumentAtIndex(idx); - } - return llvm::StringRef(); + if (idx >= m_args.size()) + return llvm::StringRef(); + + m_args[idx] = arg_str; + if (idx >= m_args_quote_char.size()) + m_args_quote_char.resize(idx + 1); + m_args_quote_char[idx] = quote_char; + return GetArgumentAtIndex(idx); } void Args::DeleteArgumentAtIndex(size_t idx) { - // Since we are using a std::list to hold onto the copied C string and - // we don't have direct access to the elements, we have to iterate to - // find the value. - arg_sstr_collection::iterator pos, end = m_args.end(); - size_t i = idx; - for (pos = m_args.begin(); i > 0 && pos != end; ++pos) - --i; - - if (pos != end) { - m_args.erase(pos); - assert(idx < m_argv.size() - 1); - m_argv.erase(m_argv.begin() + idx); - if (idx < m_args_quote_char.size()) - m_args_quote_char.erase(m_args_quote_char.begin() + idx); - } + if (idx >= m_args.size()) + return; + + m_args.erase(m_args.begin() + idx); + if (idx < m_args_quote_char.size()) + m_args_quote_char.erase(m_args_quote_char.begin() + idx); } -void Args::SetArguments(size_t argc, const char **argv) { - // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is - // no need to clear it here. +void Args::SetArguments(llvm::ArrayRef<const char *> args) { m_args.clear(); m_args_quote_char.clear(); - // First copy each string - for (size_t i = 0; i < argc; ++i) { - m_args.push_back(argv[i]); - if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`')) - m_args_quote_char.push_back(argv[i][0]); + for (const char *arg : args) { + m_args.push_back(arg); + if ((arg[0] == '\'') || (arg[0] == '"') || (arg[0] == '`')) + m_args_quote_char.push_back(arg[0]); else m_args_quote_char.push_back('\0'); } - - UpdateArgvFromArgs(); } -void Args::SetArguments(const char **argv) { - // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is - // no need to clear it here. - m_args.clear(); - m_args_quote_char.clear(); - - if (argv) { - // First copy each string - for (size_t i = 0; argv[i]; ++i) { - m_args.push_back(argv[i]); - if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`')) - m_args_quote_char.push_back(argv[i][0]); - else - m_args_quote_char.push_back('\0'); - } - } - - UpdateArgvFromArgs(); +void Args::SetArguments(const Args &other) { + m_args = other.m_args; + m_args_quote_char = other.m_args_quote_char; } Error Args::ParseOptions(Options &options, ExecutionContext *execution_context, @@ -509,11 +422,13 @@ std::unique_lock<std::mutex> lock; OptionParser::Prepare(lock); int val; + std::vector<const char *> args; + GetArgumentVector(args); + args.insert(args.begin(), "dummy"); while (1) { int long_options_index = -1; - val = - OptionParser::Parse(GetArgumentCount(), GetArgumentVector(), - sstr.GetData(), long_options, &long_options_index); + val = OptionParser::Parse(args.size(), &args[0], sstr.GetData(), + long_options, &long_options_index); if (val == -1) break; @@ -591,15 +506,14 @@ break; } - // Update our ARGV now that get options has consumed all the options - m_argv.erase(m_argv.begin(), m_argv.begin() + OptionParser::GetOptionIndex()); - UpdateArgsAfterOptionParsing(); + args.erase(args.begin(), args.begin() + OptionParser::GetOptionIndex()); + UpdateArgsAfterOptionParsing(llvm::makeArrayRef(args)); + return error; } void Args::Clear() { m_args.clear(); - m_argv.clear(); m_args_quote_char.clear(); } @@ -853,6 +767,14 @@ return fail_value; } +llvm::ArrayRef<const char *> Args::ArgvToArrayRef(const char **args) { + int argc = 0; + const char **argv2 = args; + while (*argv2) + ++argc; + return llvm::ArrayRef<const char *>(args, argc); +} + lldb::ScriptLanguage Args::StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value, bool *success_ptr) { @@ -1097,11 +1019,12 @@ std::unique_lock<std::mutex> lock; OptionParser::Prepare(lock); int val; + std::vector<const char *> args; + GetArgumentVector(args); while (1) { int long_options_index = -1; - val = - OptionParser::Parse(GetArgumentCount(), GetArgumentVector(), - sstr.GetData(), long_options, &long_options_index); + val = OptionParser::Parse(args.size(), &args[0], sstr.GetData(), + long_options, &long_options_index); if (val == -1) break; @@ -1267,19 +1190,18 @@ // doesn't // change the one we have. - std::vector<const char *> dummy_vec( - GetArgumentVector(), GetArgumentVector() + GetArgumentCount() + 1); + std::vector<const char *> dummy_vec; + GetArgumentVector(dummy_vec); bool failed_once = false; uint32_t dash_dash_pos = -1; while (1) { bool missing_argument = false; int long_options_index = -1; - val = OptionParser::Parse( - dummy_vec.size() - 1, const_cast<char *const *>(&dummy_vec.front()), - sstr.GetData(), long_options, &long_options_index); + val = OptionParser::Parse(dummy_vec.size(), &dummy_vec[0], sstr.GetData(), + long_options, &long_options_index); if (val == -1) { // When we're completing a "--" which is the last option on line, Index: source/Host/common/OptionParser.cpp =================================================================== --- source/Host/common/OptionParser.cpp +++ source/Host/common/OptionParser.cpp @@ -28,8 +28,9 @@ void OptionParser::EnableError(bool error) { opterr = error ? 1 : 0; } -int OptionParser::Parse(int argc, char *const argv[], const char *optstring, - const Option *longopts, int *longindex) { +int OptionParser::Parse(int argc, char const *const argv[], + const char *optstring, const Option *longopts, + int *longindex) { std::vector<option> opts; while (longopts->definition != nullptr) { option opt; @@ -41,7 +42,8 @@ ++longopts; } opts.push_back(option()); - return getopt_long_only(argc, argv, optstring, &opts[0], longindex); + return getopt_long_only(argc, const_cast<char *const *>(argv), optstring, + &opts[0], longindex); } char *OptionParser::GetOptionArgument() { return optarg; } Index: source/Host/common/FileSpec.cpp =================================================================== --- source/Host/common/FileSpec.cpp +++ source/Host/common/FileSpec.cpp @@ -37,6 +37,7 @@ #include "lldb/Host/Host.h" #include "lldb/Utility/CleanUp.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/FileSystem.h" @@ -228,27 +229,27 @@ #endif } -size_t FileSpec::ResolvePartialUsername(const char *partial_name, +size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name, StringList &matches) { #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER size_t extant_entries = matches.GetSize(); setpwent(); struct passwd *user_entry; - const char *name_start = partial_name + 1; + partial_name = partial_name.drop_front(); std::set<std::string> name_list; while ((user_entry = getpwent()) != NULL) { - if (strstr(user_entry->pw_name, name_start) == user_entry->pw_name) { + if (llvm::StringRef(user_entry->pw_name).startswith(partial_name)) { std::string tmp_buf("~"); tmp_buf.append(user_entry->pw_name); tmp_buf.push_back('/'); name_list.insert(tmp_buf); } } - std::set<std::string>::iterator pos, end = name_list.end(); - for (pos = name_list.begin(); pos != end; pos++) { - matches.AppendString((*pos).c_str()); + + for (auto &name : name_list) { + matches.AppendString(name); } return matches.GetSize() - extant_entries; #else @@ -284,23 +285,15 @@ // Default constructor that can take an optional full path to a // file on disk. //------------------------------------------------------------------ -FileSpec::FileSpec(const char *pathname, bool resolve_path, PathSyntax syntax) +FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, PathSyntax syntax) : m_directory(), m_filename(), m_is_resolved(false), m_syntax(syntax) { - if (pathname && pathname[0]) - SetFile(pathname, resolve_path, syntax); + SetFile(path, resolve_path, syntax); } -FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch) - : FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows() - ? ePathSyntaxWindows - : ePathSyntaxPosix} {} - -FileSpec::FileSpec(const std::string &path, bool resolve_path, - PathSyntax syntax) - : FileSpec{path.c_str(), resolve_path, syntax} {} - -FileSpec::FileSpec(const std::string &path, bool resolve_path, ArchSpec arch) - : FileSpec{path.c_str(), resolve_path, arch} {} +FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch) + : FileSpec{path, resolve_path, arch.GetTriple().isOSWindows() + ? ePathSyntaxWindows + : ePathSyntaxPosix} {} //------------------------------------------------------------------ // Copy constructor @@ -340,15 +333,20 @@ // be split up into a directory and filename and stored as uniqued // string values for quick comparison and efficient memory usage. //------------------------------------------------------------------ -void FileSpec::SetFile(const char *pathname, bool resolve, PathSyntax syntax) { +void FileSpec::SetFile(llvm::StringRef pathname, bool resolve, + PathSyntax syntax) { + // CLEANUP: Use StringRef for string handling. This function is kind of a + // mess and the unclear semantics of RootDirStart and ParentPathEnd make + // it very difficult to understand this function. There's no reason this + // function should be particularly complicated or difficult to understand. m_filename.Clear(); m_directory.Clear(); m_is_resolved = false; m_syntax = (syntax == ePathSyntaxHostNative) ? FileSystem::GetNativePathSyntax() : syntax; - if (pathname == NULL || pathname[0] == '\0') + if (pathname.empty()) return; llvm::SmallString<64> resolved(pathname); @@ -382,20 +380,10 @@ : resolve_path_ref.substr(filename_begin)); } -void FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch) { - return SetFile(pathname, resolve, arch.GetTriple().isOSWindows() - ? ePathSyntaxWindows - : ePathSyntaxPosix); -} - -void FileSpec::SetFile(const std::string &pathname, bool resolve, - PathSyntax syntax) { - return SetFile(pathname.c_str(), resolve, syntax); -} - -void FileSpec::SetFile(const std::string &pathname, bool resolve, - ArchSpec arch) { - return SetFile(pathname.c_str(), resolve, arch); +void FileSpec::SetFile(llvm::StringRef path, bool resolve, ArchSpec arch) { + return SetFile(path, resolve, arch.GetTriple().isOSWindows() + ? ePathSyntaxWindows + : ePathSyntaxPosix); } //---------------------------------------------------------------------- @@ -692,6 +680,7 @@ } bool FileSpec::ResolveExecutableLocation() { + // CLEANUP: Use StringRef for string handling. if (!m_directory) { const char *file_cstr = m_filename.GetCString(); if (file_cstr) { @@ -1023,9 +1012,11 @@ } FileSpec::EnumerateDirectoryResult -FileSpec::ForEachItemInDirectory(const char *dir_path, +FileSpec::ForEachItemInDirectory(llvm::StringRef dir_path, DirectoryCallback const &callback) { - if (dir_path && dir_path[0]) { + if (dir_path.empty()) + return eEnumerateDirectoryResultNext; + #ifdef _WIN32 std::string szDir(dir_path); szDir += "\\*"; @@ -1065,55 +1056,51 @@ continue; } - std::vector<char> child_path(PATH_MAX); - const int child_path_len = - ::snprintf(child_path.data(), child_path.size(), "%s\\%s", dir_path, - fileName.c_str()); - if (child_path_len < (int)(child_path.size() - 1)) { - // Don't resolve the file type or path - FileSpec child_path_spec(child_path.data(), false); - - EnumerateDirectoryResult result = callback(file_type, child_path_spec); - - switch (result) { - case eEnumerateDirectoryResultNext: - // Enumerate next entry in the current directory. We just - // exit this switch and will continue enumerating the - // current directory as we currently are... - break; + std::string child_path = llvm::join_items("\\", dir_path, fileName); + // Don't resolve the file type or path + FileSpec child_path_spec(child_path.data(), false); - case eEnumerateDirectoryResultEnter: // Recurse into the current entry - // if it is a directory or symlink, - // or next if not - if (FileSpec::ForEachItemInDirectory(child_path.data(), callback) == - eEnumerateDirectoryResultQuit) { - // The subdirectory returned Quit, which means to - // stop all directory enumerations at all levels. - return eEnumerateDirectoryResultQuit; - } - break; + EnumerateDirectoryResult result = callback(file_type, child_path_spec); - case eEnumerateDirectoryResultExit: // Exit from the current directory - // at the current level. - // Exit from this directory level and tell parent to - // keep enumerating. - return eEnumerateDirectoryResultNext; + switch (result) { + case eEnumerateDirectoryResultNext: + // Enumerate next entry in the current directory. We just + // exit this switch and will continue enumerating the + // current directory as we currently are... + break; - case eEnumerateDirectoryResultQuit: // Stop directory enumerations at - // any level + case eEnumerateDirectoryResultEnter: // Recurse into the current entry + // if it is a directory or symlink, + // or next if not + if (FileSpec::ForEachItemInDirectory(child_path.data(), callback) == + eEnumerateDirectoryResultQuit) { + // The subdirectory returned Quit, which means to + // stop all directory enumerations at all levels. return eEnumerateDirectoryResultQuit; } + break; + + case eEnumerateDirectoryResultExit: // Exit from the current directory + // at the current level. + // Exit from this directory level and tell parent to + // keep enumerating. + return eEnumerateDirectoryResultNext; + + case eEnumerateDirectoryResultQuit: // Stop directory enumerations at + // any level + return eEnumerateDirectoryResultQuit; } } while (FindNextFileW(hFind, &ffd) != 0); FindClose(hFind); #else - lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_path), NULL, - closedir); - if (dir_path_dir.is_valid()) { - char dir_path_last_char = dir_path[strlen(dir_path) - 1]; + std::string dir_string(dir_path); + lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_string.c_str()), + NULL, closedir); + if (dir_path_dir.is_valid()) { + char dir_path_last_char = dir_path.back(); - long path_max = fpathconf(dirfd(dir_path_dir.get()), _PC_NAME_MAX); + long path_max = fpathconf(dirfd(dir_path_dir.get()), _PC_NAME_MAX); #if defined(__APPLE_) && defined(__DARWIN_MAXPATHLEN) if (path_max < __DARWIN_MAXPATHLEN) path_max = __DARWIN_MAXPATHLEN; @@ -1169,18 +1156,13 @@ #endif } - char child_path[PATH_MAX]; - + std::string child_path; // Don't make paths with "/foo//bar", that just confuses everybody. - int child_path_len; if (dir_path_last_char == '/') - child_path_len = ::snprintf(child_path, sizeof(child_path), "%s%s", - dir_path, dp->d_name); + child_path = llvm::join_items("", dir_path, dp->d_name); else - child_path_len = ::snprintf(child_path, sizeof(child_path), "%s/%s", - dir_path, dp->d_name); + child_path = llvm::join_items('/', dir_path, dp->d_name); - if (child_path_len < (int)(sizeof(child_path) - 1)) { // Don't resolve the file type or path FileSpec child_path_spec(child_path, false); @@ -1221,21 +1203,19 @@ free(buf); return eEnumerateDirectoryResultQuit; } - } } if (buf) { free(buf); } } #endif - } // By default when exiting a directory, we tell the parent enumeration // to continue enumerating. return eEnumerateDirectoryResultNext; } FileSpec::EnumerateDirectoryResult -FileSpec::EnumerateDirectory(const char *dir_path, bool find_directories, +FileSpec::EnumerateDirectory(llvm::StringRef dir_path, bool find_directories, bool find_files, bool find_other, EnumerateDirectoryCallbackType callback, void *callback_baton) { @@ -1261,13 +1241,15 @@ }); } -FileSpec FileSpec::CopyByAppendingPathComponent(const char *new_path) const { +FileSpec +FileSpec::CopyByAppendingPathComponent(llvm::StringRef component) const { FileSpec ret = *this; - ret.AppendPathComponent(new_path); + ret.AppendPathComponent(component); return ret; } FileSpec FileSpec::CopyByRemovingLastPathComponent() const { + // CLEANUP: Use StringRef for string handling. const bool resolve = false; if (m_filename.IsEmpty() && m_directory.IsEmpty()) return FileSpec("", resolve); @@ -1291,6 +1273,7 @@ } ConstString FileSpec::GetLastPathComponent() const { + // CLEANUP: Use StringRef for string handling. if (m_filename) return m_filename; if (m_directory) { @@ -1321,68 +1304,65 @@ return ConstString(); } -void FileSpec::PrependPathComponent(const char *new_path) { - if (!new_path) +void FileSpec::PrependPathComponent(llvm::StringRef component) { + if (component.empty()) return; + const bool resolve = false; if (m_filename.IsEmpty() && m_directory.IsEmpty()) { - SetFile(new_path, resolve); + SetFile(component, resolve); return; } - StreamString stream; + + char sep = GetPrefferedPathSeparator(m_syntax); + std::string result; if (m_filename.IsEmpty()) - stream.Printf("%s/%s", new_path, m_directory.GetCString()); + result = llvm::join_items(sep, component, m_directory.GetStringRef()); else if (m_directory.IsEmpty()) - stream.Printf("%s/%s", new_path, m_filename.GetCString()); + result = llvm::join_items(sep, component, m_filename.GetStringRef()); else - stream.Printf("%s/%s/%s", new_path, m_directory.GetCString(), - m_filename.GetCString()); - SetFile(stream.GetData(), resolve); -} + result = llvm::join_items(sep, component, m_directory.GetStringRef(), + m_filename.GetStringRef()); -void FileSpec::PrependPathComponent(const std::string &new_path) { - return PrependPathComponent(new_path.c_str()); + SetFile(result, resolve); } void FileSpec::PrependPathComponent(const FileSpec &new_path) { return PrependPathComponent(new_path.GetPath(false)); } -void FileSpec::AppendPathComponent(const char *new_path) { - if (!new_path) +void FileSpec::AppendPathComponent(llvm::StringRef component) { + if (component.empty()) return; - StreamString stream; + std::string result; if (!m_directory.IsEmpty()) { - stream.PutCString(m_directory.GetCString()); + result += m_directory.GetStringRef(); if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax)) - stream.PutChar(GetPrefferedPathSeparator(m_syntax)); + result += GetPrefferedPathSeparator(m_syntax); } if (!m_filename.IsEmpty()) { - stream.PutCString(m_filename.GetCString()); + result += m_filename.GetStringRef(); if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax)) - stream.PutChar(GetPrefferedPathSeparator(m_syntax)); + result += GetPrefferedPathSeparator(m_syntax); } - while (IsPathSeparator(new_path[0], m_syntax)) - new_path++; + component = component.drop_while( + [this](char c) { return IsPathSeparator(c, m_syntax); }); - stream.PutCString(new_path); + result += component; - const bool resolve = false; - SetFile(stream.GetData(), resolve, m_syntax); -} - -void FileSpec::AppendPathComponent(const std::string &new_path) { - return AppendPathComponent(new_path.c_str()); + SetFile(result, false, m_syntax); } void FileSpec::AppendPathComponent(const FileSpec &new_path) { return AppendPathComponent(new_path.GetPath(false)); } void FileSpec::RemoveLastPathComponent() { + // CLEANUP: Use StringRef for string handling. + const bool resolve = false; if (m_filename.IsEmpty() && m_directory.IsEmpty()) { SetFile("", resolve); Index: source/Commands/CommandObjectLog.cpp =================================================================== --- source/Commands/CommandObjectLog.cpp +++ source/Commands/CommandObjectLog.cpp @@ -190,9 +190,11 @@ m_options.log_file.GetPath(log_file, sizeof(log_file)); else log_file[0] = '\0'; + std::vector<const char *> argv; + args.GetArgumentVector(argv); bool success = m_interpreter.GetDebugger().EnableLog( - channel.c_str(), args.GetConstArgumentVector(), log_file, - m_options.log_options, result.GetErrorStream()); + channel.c_str(), &argv[0], log_file, m_options.log_options, + result.GetErrorStream()); if (success) result.SetStatus(eReturnStatusSuccessFinishNoResult); else @@ -250,18 +252,19 @@ std::string channel(args.GetArgumentAtIndex(0)); args.Shift(); // Shift off the channel + std::vector<const char *> argv; + args.GetArgumentVector(argv); + if (Log::GetLogChannelCallbacks(ConstString(channel.c_str()), log_callbacks)) { - log_callbacks.disable(args.GetConstArgumentVector(), - &result.GetErrorStream()); + log_callbacks.disable(&argv[0], &result.GetErrorStream()); result.SetStatus(eReturnStatusSuccessFinishNoResult); } else if (channel == "all") { Log::DisableAllLogChannels(&result.GetErrorStream()); } else { LogChannelSP log_channel_sp(LogChannel::FindPlugin(channel.c_str())); if (log_channel_sp) { - log_channel_sp->Disable(args.GetConstArgumentVector(), - &result.GetErrorStream()); + log_channel_sp->Disable(&argv[0], &result.GetErrorStream()); result.SetStatus(eReturnStatusSuccessFinishNoResult); } else result.AppendErrorWithFormat("Invalid log channel '%s'.\n", Index: source/Commands/CommandObjectBreakpoint.cpp =================================================================== --- source/Commands/CommandObjectBreakpoint.cpp +++ source/Commands/CommandObjectBreakpoint.cpp @@ -2423,8 +2423,9 @@ // NOW, convert the list of breakpoint id strings in TEMP_ARGS into an actual // BreakpointIDList: - valid_ids->InsertStringArray(temp_args.GetConstArgumentVector(), - temp_args.GetArgumentCount(), result); + std::vector<const char *> argv; + temp_args.GetArgumentVector(argv); + valid_ids->InsertStringArray(&argv[0], temp_args.GetArgumentCount(), result); // At this point, all of the breakpoint ids that the user passed in have been // converted to breakpoint IDs Index: source/API/SBTarget.cpp =================================================================== --- source/API/SBTarget.cpp +++ source/API/SBTarget.cpp @@ -287,9 +287,10 @@ if (exe_module) launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true); if (argv) - launch_info.GetArguments().AppendArguments(argv); + launch_info.GetArguments().AppendArguments(Args::ArgvToArrayRef(argv)); if (envp) - launch_info.GetEnvironmentEntries().SetArguments(envp); + launch_info.GetEnvironmentEntries().SetArguments( + Args::ArgvToArrayRef(envp)); if (listener.IsValid()) launch_info.SetListener(listener.GetSP()); Index: source/API/SBProcess.cpp =================================================================== --- source/API/SBProcess.cpp +++ source/API/SBProcess.cpp @@ -136,9 +136,10 @@ if (exe_module) launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true); if (argv) - launch_info.GetArguments().AppendArguments(argv); + launch_info.GetArguments().AppendArguments(Args::ArgvToArrayRef(argv)); if (envp) - launch_info.GetEnvironmentEntries().SetArguments(envp); + launch_info.GetEnvironmentEntries().SetArguments( + Args::ArgvToArrayRef(envp)); error.SetError(process_sp->Launch(launch_info)); } else { error.SetErrorString("must be in eStateConnected to call RemoteLaunch"); Index: source/API/SBLaunchInfo.cpp =================================================================== --- source/API/SBLaunchInfo.cpp +++ source/API/SBLaunchInfo.cpp @@ -19,8 +19,9 @@ SBLaunchInfo::SBLaunchInfo(const char **argv) : m_opaque_sp(new ProcessLaunchInfo()) { m_opaque_sp->GetFlags().Reset(eLaunchFlagDebug | eLaunchFlagDisableASLR); - if (argv && argv[0]) - m_opaque_sp->GetArguments().SetArguments(argv); + if (argv) { + m_opaque_sp->GetArguments().SetArguments(Args::ArgvToArrayRef(argv)); + } } SBLaunchInfo::~SBLaunchInfo() {} @@ -71,15 +72,11 @@ } void SBLaunchInfo::SetArguments(const char **argv, bool append) { - if (append) { - if (argv) - m_opaque_sp->GetArguments().AppendArguments(argv); - } else { - if (argv) - m_opaque_sp->GetArguments().SetArguments(argv); - else - m_opaque_sp->GetArguments().Clear(); - } + if (!append) + m_opaque_sp->GetArguments().Clear(); + + if (argv) + m_opaque_sp->GetArguments().AppendArguments(Args::ArgvToArrayRef(argv)); } uint32_t SBLaunchInfo::GetNumEnvironmentEntries() { @@ -91,15 +88,12 @@ } void SBLaunchInfo::SetEnvironmentEntries(const char **envp, bool append) { - if (append) { - if (envp) - m_opaque_sp->GetEnvironmentEntries().AppendArguments(envp); - } else { - if (envp) - m_opaque_sp->GetEnvironmentEntries().SetArguments(envp); - else - m_opaque_sp->GetEnvironmentEntries().Clear(); - } + if (!append) + m_opaque_sp->GetEnvironmentEntries().Clear(); + + if (envp) + m_opaque_sp->GetEnvironmentEntries().AppendArguments( + Args::ArgvToArrayRef(envp)); } void SBLaunchInfo::Clear() { m_opaque_sp->Clear(); } Index: source/API/SBCommandInterpreter.cpp =================================================================== --- source/API/SBCommandInterpreter.cpp +++ source/API/SBCommandInterpreter.cpp @@ -115,8 +115,9 @@ SBCommandReturnObject sb_return(&result); SBCommandInterpreter sb_interpreter(&m_interpreter); SBDebugger debugger_sb(m_interpreter.GetDebugger().shared_from_this()); - bool ret = m_backend->DoExecute( - debugger_sb, (char **)command.GetArgumentVector(), sb_return); + std::vector<const char *> argv; + command.GetArgumentVector(argv); + bool ret = m_backend->DoExecute(debugger_sb, &argv[0], sb_return); sb_return.Release(); return ret; } Index: include/lldb/Interpreter/Args.h =================================================================== --- include/lldb/Interpreter/Args.h +++ include/lldb/Interpreter/Args.h @@ -18,7 +18,9 @@ #include <vector> // Other libraries and framework includes +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/StringRef.h" + // Project includes #include "lldb/Core/Error.h" #include "lldb/Host/OptionParser.h" @@ -146,26 +148,10 @@ /// \endcode /// /// @return - /// An array of NULL terminated C string argument pointers that - /// also has a terminating NULL C string pointer - //------------------------------------------------------------------ - char **GetArgumentVector(); - - //------------------------------------------------------------------ - /// Gets the argument vector. - /// - /// The value returned by this function can be used by any function - /// that takes and vector. The return value is just like \a argv - /// in the standard C entry point function: - /// \code - /// int main (int argc, const char **argv); - /// \endcode - /// - /// @return /// An array of NULL terminate C string argument pointers that /// also has a terminating NULL C string pointer //------------------------------------------------------------------ - const char **GetConstArgumentVector() const; + void GetArgumentVector(std::vector<const char *> &args) const; //------------------------------------------------------------------ /// Appends a new argument to the end of the list argument list. @@ -183,52 +169,7 @@ char quote_char = '\0'); void AppendArguments(const Args &rhs); - - void AppendArguments(const char **argv); - - // Delete const char* versions of StringRef functions. Normally this would - // not be necessary, as const char * is implicitly convertible to StringRef. - // However, since the use of const char* is so pervasive, and since StringRef - // will assert if you try to construct one from nullptr, this allows the - // compiler to catch instances of the function being invoked with a - // const char *, allowing us to replace them with explicit conversions at each - // call-site. This ensures that no callsites slip through the cracks where - // we would be trying to implicitly convert from nullptr, since it will force - // us to evaluate and explicitly convert each one. - // - // Once StringRef use becomes more pervasive, there will be fewer - // implicit conversions because we will be using StringRefs across the whole - // pipeline, so we won't have to have this "glue" that converts between the - // two, and at that point it becomes easy to just make sure you don't pass - // nullptr into the function on the odd occasion that you do pass a - // const char *. - // Call-site fixing methodology: - // 1. If you know the string cannot be null (e.g. it's a const char[], or - // it's been checked for null), use llvm::StringRef(ptr). - // 2. If you don't know if it can be null (e.g. it's returned from a - // function whose semantics are unclear), use - // llvm::StringRef::withNullAsEmpty(ptr). - // 3. If it's .c_str() of a std::string, just pass the std::string directly. - // 4. If it's .str().c_str() of a StringRef, just pass the StringRef - // directly. - void ReplaceArgumentAtIndex(size_t, const char *, char = '\0') = delete; - void AppendArgument(const char *arg_str, char quote_char = '\0') = delete; - void InsertArgumentAtIndex(size_t, const char *, char = '\0') = delete; - static bool StringToBoolean(const char *, bool, bool *) = delete; - static lldb::ScriptLanguage - StringToScriptLanguage(const char *, lldb::ScriptLanguage, bool *) = delete; - static lldb::Encoding - StringToEncoding(const char *, - lldb::Encoding = lldb::eEncodingInvalid) = delete; - static uint32_t StringToGenericRegister(const char *) = delete; - static bool StringToVersion(const char *, uint32_t &, uint32_t &, - uint32_t &) = delete; - const char *Unshift(const char *, char = '\0') = delete; - void AddOrReplaceEnvironmentVariable(const char *, const char *) = delete; - bool ContainsEnvironmentVariable(const char *, - size_t * = nullptr) const = delete; - static int64_t StringToOptionEnum(const char *, OptionEnumValueElement *, - int32_t, Error &) = delete; + void AppendArguments(llvm::ArrayRef<const char *> args); //------------------------------------------------------------------ /// Insert the argument value at index \a idx to \a arg_cstr. @@ -287,9 +228,8 @@ // // FIXME: Handle the quote character somehow. //------------------------------------------------------------------ - void SetArguments(size_t argc, const char **argv); - - void SetArguments(const char **argv); + void SetArguments(llvm::ArrayRef<const char *> args); + void SetArguments(const Args &other); //------------------------------------------------------------------ /// Shifts the first argument C string value of the array off the @@ -413,6 +353,8 @@ OptionEnumValueElement *enum_values, int32_t fail_value, Error &error); + static llvm::ArrayRef<const char *> ArgvToArrayRef(const char **args); + static lldb::ScriptLanguage StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value, bool *success_ptr); @@ -497,16 +439,12 @@ //------------------------------------------------------------------ // Classes that inherit from Args can see and modify these //------------------------------------------------------------------ - typedef std::list<std::string> arg_sstr_collection; - typedef std::vector<const char *> arg_cstr_collection; + typedef std::vector<std::string> arg_sstr_collection; typedef std::vector<char> arg_quote_char_collection; arg_sstr_collection m_args; - arg_cstr_collection m_argv; ///< The current argument vector. arg_quote_char_collection m_args_quote_char; - void UpdateArgsAfterOptionParsing(); - - void UpdateArgvFromArgs(); + void UpdateArgsAfterOptionParsing(llvm::ArrayRef<const char *> new_argv); llvm::StringRef ParseSingleArgument(llvm::StringRef command); }; Index: include/lldb/Host/OptionParser.h =================================================================== --- include/lldb/Host/OptionParser.h +++ include/lldb/Host/OptionParser.h @@ -36,7 +36,7 @@ static void EnableError(bool error); - static int Parse(int argc, char *const argv[], const char *optstring, + static int Parse(int argc, char const *const argv[], const char *optstring, const Option *longopts, int *longindex); static char *GetOptionArgument(); Index: include/lldb/Host/FileSpec.h =================================================================== --- include/lldb/Host/FileSpec.h +++ include/lldb/Host/FileSpec.h @@ -80,16 +80,10 @@ /// /// @see FileSpec::SetFile (const char *path, bool resolve) //------------------------------------------------------------------ - // TODO: Convert this constructor to use a StringRef. - explicit FileSpec(const char *path, bool resolve_path, + explicit FileSpec(llvm::StringRef path, bool resolve_path, PathSyntax syntax = ePathSyntaxHostNative); - explicit FileSpec(const char *path, bool resolve_path, ArchSpec arch); - - explicit FileSpec(const std::string &path, bool resolve_path, - PathSyntax syntax = ePathSyntaxHostNative); - - explicit FileSpec(const std::string &path, bool resolve_path, ArchSpec arch); + explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch); //------------------------------------------------------------------ /// Copy constructor @@ -657,15 +651,10 @@ /// If \b true, then we will try to resolve links the path using /// the static FileSpec::Resolve. //------------------------------------------------------------------ - void SetFile(const char *path, bool resolve_path, - PathSyntax syntax = ePathSyntaxHostNative); - - void SetFile(const char *path, bool resolve_path, ArchSpec arch); - - void SetFile(const std::string &path, bool resolve_path, + void SetFile(llvm::StringRef path, bool resolve_path, PathSyntax syntax = ePathSyntaxHostNative); - void SetFile(const std::string &path, bool resolve_path, ArchSpec arch); + void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch); bool IsResolved() const { return m_is_resolved; } @@ -709,20 +698,13 @@ //------------------------------------------------------------------ static void Resolve(llvm::SmallVectorImpl<char> &path); - FileSpec CopyByAppendingPathComponent(const char *new_path) const; - + FileSpec CopyByAppendingPathComponent(llvm::StringRef component) const; FileSpec CopyByRemovingLastPathComponent() const; - void PrependPathComponent(const char *new_path); - - void PrependPathComponent(const std::string &new_path); - + void PrependPathComponent(llvm::StringRef component); void PrependPathComponent(const FileSpec &new_path); - void AppendPathComponent(const char *new_path); - - void AppendPathComponent(const std::string &new_path); - + void AppendPathComponent(llvm::StringRef component); void AppendPathComponent(const FileSpec &new_path); void RemoveLastPathComponent(); @@ -746,7 +728,7 @@ //------------------------------------------------------------------ static void ResolveUsername(llvm::SmallVectorImpl<char> &path); - static size_t ResolvePartialUsername(const char *partial_name, + static size_t ResolvePartialUsername(llvm::StringRef partial_name, StringList &matches); enum EnumerateDirectoryResult { @@ -763,7 +745,7 @@ void *baton, FileType file_type, const FileSpec &spec); static EnumerateDirectoryResult - EnumerateDirectory(const char *dir_path, bool find_directories, + EnumerateDirectory(llvm::StringRef dir_path, bool find_directories, bool find_files, bool find_other, EnumerateDirectoryCallbackType callback, void *callback_baton); @@ -773,7 +755,7 @@ DirectoryCallback; static EnumerateDirectoryResult - ForEachItemInDirectory(const char *dir_path, + ForEachItemInDirectory(llvm::StringRef dir_path, DirectoryCallback const &callback); protected: Index: include/lldb/API/SBCommandInterpreter.h =================================================================== --- include/lldb/API/SBCommandInterpreter.h +++ include/lldb/API/SBCommandInterpreter.h @@ -233,7 +233,8 @@ public: virtual ~SBCommandPluginInterface() = default; - virtual bool DoExecute(lldb::SBDebugger /*debugger*/, char ** /*command*/, + virtual bool DoExecute(lldb::SBDebugger /*debugger*/, + const char ** /*command*/, lldb::SBCommandReturnObject & /*result*/) { return false; } Index: examples/plugins/commands/fooplugin.cpp =================================================================== --- examples/plugins/commands/fooplugin.cpp +++ examples/plugins/commands/fooplugin.cpp @@ -25,8 +25,8 @@ class ChildCommand : public lldb::SBCommandPluginInterface { public: - virtual bool DoExecute(lldb::SBDebugger debugger, char **command, - lldb::SBCommandReturnObject &result) { + bool DoExecute(lldb::SBDebugger debugger, const char **command, + lldb::SBCommandReturnObject &result) override { if (command) { const char *arg = *command; while (arg) {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits