From 2fc6b024b981830ae7d1da09a5a8b5e19ca3c6ba Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Fri, 17 May 2019 22:53:04 +0000 Subject: [PATCH] [CommandInterpreter] Refactor SourceInitFile I was looking at the current implementation of SourceInitFile and there were a few things that made this function hard to read: * The code to find the ~/.lldbinit file is duplicated across the cwd and non-cwd branch. * The ./.lldbinit is once computed by resolving .lldbinit and once by resolving ./.lldbinit. * It wasn't clear to me what happened when you're sourcing the .lldbinit file in the current working directory. Apparently we do nothing when we property to control that is set to warn (makes sense) and we don't care when the property is set to true (debatable). * There were at least two branches where the status of the CommandReturnObject were not set. This patch attempts to simplify that code. Differential revision: https://reviews.llvm.org/D61994 llvm-svn: 361080 --- .../lldb/Interpreter/CommandInterpreter.h | 5 +- lldb/source/API/SBCommandInterpreter.cpp | 4 +- .../source/Interpreter/CommandInterpreter.cpp | 205 ++++++++++-------- 3 files changed, 121 insertions(+), 93 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index 85285698db48..efbd849635a7 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -212,7 +212,8 @@ public: return GetStaticBroadcasterClass(); } - void SourceInitFile(bool in_cwd, CommandReturnObject &result); + void SourceInitFileCwd(CommandReturnObject &result); + void SourceInitFileHome(CommandReturnObject &result); bool AddCommand(llvm::StringRef name, const lldb::CommandObjectSP &cmd_sp, bool can_replace); @@ -533,6 +534,8 @@ protected: private: Status PreprocessCommand(std::string &command); + void SourceInitFile(FileSpec file, CommandReturnObject &result); + // Completely resolves aliases and abbreviations, returning a pointer to the // final command object and updating command_line to the fully substituted // and translated command. diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp index e6d7ac61817b..c07dffce0baa 100644 --- a/lldb/source/API/SBCommandInterpreter.cpp +++ b/lldb/source/API/SBCommandInterpreter.cpp @@ -540,7 +540,7 @@ void SBCommandInterpreter::SourceInitFileInHomeDirectory( std::unique_lock lock; if (target_sp) lock = std::unique_lock(target_sp->GetAPIMutex()); - m_opaque_ptr->SourceInitFile(false, result.ref()); + m_opaque_ptr->SourceInitFileHome(result.ref()); } else { result->AppendError("SBCommandInterpreter is not valid"); result->SetStatus(eReturnStatusFailed); @@ -559,7 +559,7 @@ void SBCommandInterpreter::SourceInitFileInCurrentWorkingDirectory( std::unique_lock lock; if (target_sp) lock = std::unique_lock(target_sp->GetAPIMutex()); - m_opaque_ptr->SourceInitFile(true, result.ref()); + m_opaque_ptr->SourceInitFileCwd(result.ref()); } else { result->AppendError("SBCommandInterpreter is not valid"); result->SetStatus(eReturnStatusFailed); diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index ae4ba9aac30f..f6027636bd3c 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -81,6 +81,17 @@ static constexpr bool NoGlobalSetting = true; static constexpr uintptr_t DefaultValueTrue = true; static constexpr uintptr_t DefaultValueFalse = false; static constexpr const char *NoCStrDefault = nullptr; +static constexpr const char *InitFileWarning = + "There is a .lldbinit file in the current directory which is not being " + "read.\n" + "To silence this warning without sourcing in the local .lldbinit,\n" + "add the following to the lldbinit file in your home directory:\n" + " settings set target.load-cwd-lldbinit false\n" + "To allow lldb to source .lldbinit files in the current working " + "directory,\n" + "set the value of this variable to true. Only do so if you understand " + "and\n" + "accept the security risk."; static constexpr PropertyDefinition g_properties[] = { {"expand-regex-aliases", OptionValue::eTypeBoolean, NoGlobalSetting, @@ -2091,100 +2102,114 @@ int CommandInterpreter::GetOptionArgumentPosition(const char *in_string) { return position; } -void CommandInterpreter::SourceInitFile(bool in_cwd, +static void GetHomeInitFile(llvm::SmallVectorImpl &init_file, + llvm::StringRef suffix = {}) { + std::string init_file_name = ".lldbinit"; + if (!suffix.empty()) { + init_file_name.append("-"); + init_file_name.append(suffix.str()); + } + + llvm::sys::path::home_directory(init_file); + llvm::sys::path::append(init_file, init_file_name); + + FileSystem::Instance().Resolve(init_file); +} + +static void GetCwdInitFile(llvm::SmallVectorImpl &init_file) { + llvm::StringRef s = ".lldbinit"; + init_file.assign(s.begin(), s.end()); + FileSystem::Instance().Resolve(init_file); +} + +static LoadCWDlldbinitFile ShouldLoadCwdInitFile() { + lldb::TargetPropertiesSP properties = Target::GetGlobalProperties(); + if (!properties) + return eLoadCWDlldbinitFalse; + return properties->GetLoadCWDlldbinitFile(); +} + +void CommandInterpreter::SourceInitFile(FileSpec file, CommandReturnObject &result) { - FileSpec init_file; - if (in_cwd) { - lldb::TargetPropertiesSP properties = Target::GetGlobalProperties(); - if (properties) { - // In the current working directory we don't load any program specific - // .lldbinit files, we only look for a ".lldbinit" file. - if (m_skip_lldbinit_files) - return; + assert(!m_skip_lldbinit_files); - LoadCWDlldbinitFile should_load = properties->GetLoadCWDlldbinitFile(); - if (should_load == eLoadCWDlldbinitWarn) { - FileSpec dot_lldb(".lldbinit"); - FileSystem::Instance().Resolve(dot_lldb); - llvm::SmallString<64> home_dir_path; - llvm::sys::path::home_directory(home_dir_path); - FileSpec homedir_dot_lldb(home_dir_path.c_str()); - homedir_dot_lldb.AppendPathComponent(".lldbinit"); - FileSystem::Instance().Resolve(homedir_dot_lldb); - if (FileSystem::Instance().Exists(dot_lldb) && - dot_lldb.GetDirectory() != homedir_dot_lldb.GetDirectory()) { - result.AppendErrorWithFormat( - "There is a .lldbinit file in the current directory which is not " - "being read.\n" - "To silence this warning without sourcing in the local " - ".lldbinit,\n" - "add the following to the lldbinit file in your home directory:\n" - " settings set target.load-cwd-lldbinit false\n" - "To allow lldb to source .lldbinit files in the current working " - "directory,\n" - "set the value of this variable to true. Only do so if you " - "understand and\n" - "accept the security risk."); - result.SetStatus(eReturnStatusFailed); - return; - } - } else if (should_load == eLoadCWDlldbinitTrue) { - init_file.SetFile("./.lldbinit", FileSpec::Style::native); - FileSystem::Instance().Resolve(init_file); - } - } - } else { - // If we aren't looking in the current working directory we are looking in - // the home directory. We will first see if there is an application - // specific ".lldbinit" file whose name is "~/.lldbinit" followed by a "-" - // and the name of the program. If this file doesn't exist, we fall back to - // just the "~/.lldbinit" file. We also obey any requests to not load the - // init files. - llvm::SmallString<64> home_dir_path; - llvm::sys::path::home_directory(home_dir_path); - FileSpec profilePath(home_dir_path.c_str()); - profilePath.AppendPathComponent(".lldbinit"); - std::string init_file_path = profilePath.GetPath(); - - if (!m_skip_app_init_files) { - FileSpec program_file_spec(HostInfo::GetProgramFileSpec()); - const char *program_name = program_file_spec.GetFilename().AsCString(); - - if (program_name) { - char program_init_file_name[PATH_MAX]; - ::snprintf(program_init_file_name, sizeof(program_init_file_name), - "%s-%s", init_file_path.c_str(), program_name); - init_file.SetFile(program_init_file_name, FileSpec::Style::native); - FileSystem::Instance().Resolve(init_file); - if (!FileSystem::Instance().Exists(init_file)) - init_file.Clear(); - } - } - - if (!init_file && !m_skip_lldbinit_files) - init_file.SetFile(init_file_path, FileSpec::Style::native); - } - - // If the file exists, tell HandleCommand to 'source' it; this will do the - // actual broadcasting of the commands back to any appropriate listener (see - // CommandObjectSource::Execute for more details). - - if (FileSystem::Instance().Exists(init_file)) { - const bool saved_batch = SetBatchCommandMode(true); - CommandInterpreterRunOptions options; - options.SetSilent(true); - options.SetPrintErrors(true); - options.SetStopOnError(false); - options.SetStopOnContinue(true); - - HandleCommandsFromFile(init_file, - nullptr, // Execution context - options, result); - SetBatchCommandMode(saved_batch); - } else { - // nothing to be done if the file doesn't exist + if (!FileSystem::Instance().Exists(file)) { result.SetStatus(eReturnStatusSuccessFinishNoResult); + return; } + + // Use HandleCommand to 'source' the given file; this will do the actual + // broadcasting of the commands back to any appropriate listener (see + // CommandObjectSource::Execute for more details). + const bool saved_batch = SetBatchCommandMode(true); + ExecutionContext *ctx = nullptr; + CommandInterpreterRunOptions options; + options.SetSilent(true); + options.SetPrintErrors(true); + options.SetStopOnError(false); + options.SetStopOnContinue(true); + HandleCommandsFromFile(file, ctx, options, result); + SetBatchCommandMode(saved_batch); +} + +void CommandInterpreter::SourceInitFileCwd(CommandReturnObject &result) { + if (m_skip_lldbinit_files) { + result.SetStatus(eReturnStatusSuccessFinishNoResult); + return; + } + + llvm::SmallString<128> init_file; + GetCwdInitFile(init_file); + if (!FileSystem::Instance().Exists(init_file)) { + result.SetStatus(eReturnStatusSuccessFinishNoResult); + return; + } + + LoadCWDlldbinitFile should_load = ShouldLoadCwdInitFile(); + + switch (should_load) { + case eLoadCWDlldbinitFalse: + result.SetStatus(eReturnStatusSuccessFinishNoResult); + break; + case eLoadCWDlldbinitTrue: + SourceInitFile(FileSpec(init_file.str()), result); + break; + case eLoadCWDlldbinitWarn: { + llvm::SmallString<128> home_init_file; + GetHomeInitFile(home_init_file); + if (llvm::sys::path::parent_path(init_file) == + llvm::sys::path::parent_path(home_init_file)) { + result.SetStatus(eReturnStatusSuccessFinishNoResult); + } else { + result.AppendErrorWithFormat(InitFileWarning); + result.SetStatus(eReturnStatusFailed); + } + } + } +} + +/// We will first see if there is an application specific ".lldbinit" file +/// whose name is "~/.lldbinit" followed by a "-" and the name of the program. +/// If this file doesn't exist, we fall back to just the "~/.lldbinit" file. +void CommandInterpreter::SourceInitFileHome(CommandReturnObject &result) { + if (m_skip_lldbinit_files) { + result.SetStatus(eReturnStatusSuccessFinishNoResult); + return; + } + + llvm::SmallString<128> init_file; + GetHomeInitFile(init_file); + + if (!m_skip_app_init_files) { + llvm::StringRef program_name = + HostInfo::GetProgramFileSpec().GetFilename().GetStringRef(); + llvm::SmallString<128> program_init_file; + GetHomeInitFile(program_init_file, program_name); + if (FileSystem::Instance().Exists(program_init_file)) + init_file = program_init_file; + } + + SourceInitFile(FileSpec(init_file.str()), result); } const char *CommandInterpreter::GetCommandPrefix() {