[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL361080: [CommandInterpreter] Refactor SourceInitFile 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61994?vs=200056&id=200111#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994

Files:
  lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h
  lldb/trunk/source/API/SBCommandInterpreter.cpp
  lldb/trunk/source/Interpreter/CommandInterpreter.cpp

Index: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp
@@ -81,6 +81,17 @@
 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 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
-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;
+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());
+  }
 
-  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_n

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 200056.
JDevlieghere added a comment.

Remove unneeded use of FileSpec. I wanted to do this yesterday but didn't get 
around to it...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -81,6 +81,17 @@
 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,111 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
-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;
+static llvm::SmallString<128> GetHomeInitFile(llvm::StringRef suffix = {}) {
+  std::string init_file_name = ".lldbinit";
+  if (!suffix.empty()) {
+init_file_name.append("-");
+init_file_name.append(suffix.str());
+  }
 
-  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::nati

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I think this looks better than the original. Given that the only callers of 
`SourceInitFile` pass a constant value for the `InitFileLocation` argument, 
what I'd consider also doing is getting rid of the function and the enum, and 
making SourceInitFileHome/SourceInitFileCwd the main entry points.

One day I'm going to start a thread about reducing the use of FileSpecs in the 
FileSystem api...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 199943.
JDevlieghere added a comment.

Address code review feedback


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -81,6 +81,17 @@
 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,99 +2102,120 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
+static FileSpec GetHomeInitFile(llvm::StringRef suffix = {}) {
+  std::string init_file_name = ".lldbinit";
+  if (!suffix.empty()) {
+init_file_name.append("-");
+init_file_name.append(suffix.str());
+  }
+
+  llvm::SmallString<128> buffer;
+  llvm::sys::path::home_directory(buffer);
+  llvm::sys::path::append(buffer, init_file_name);
+
+  FileSpec init_file(buffer.c_str());
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static FileSpec GetCwdInitFile() {
+  FileSpec init_file(".lldbinit");
+  FileSystem::Instance().Resolve(init_file);
+  return 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);
-

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Seems like an overall improvement to the structure of this code. At the high 
level, the structure feels easier to understand.

In D61994#1504336 , @labath wrote:

> I think this is a bit more readable. I've included some suggestions which I 
> think could make it even better.
>
> Since you're already rewriting this code, this might be a good time to raise 
> a point I wanted to bring up some day. Should we be using `FileSpec` for code 
> like this? The code already uses a combination of llvm and lldb path 
> utilities, and I would argue that we should use llvm all the way for code 
> like this (except that we go through the FileSystem abstraction for the 
> reproducer stuff). I have two reasons for that:
>
> - FileSpec is designed for efficient matching of abstract file paths which 
> may not even exist on the host system. As such, this code will result in a 
> bunch of strings being added to the global string pool for no reason. And in 
> this case, you're definitely working files which do exist (or may exist) on 
> the host. In fact, FileSpec now contains code which performs path 
> simplifications which are not completely sound given a concrete filesystem -- 
> it will simplify a path like `bar/../foo` to `foo`, which is not correct if 
> `bar` is a symlink.


+1

> - Since we started supporting windows paths, the FileSpec class offers more 
> freedom than it is needed here. Specifically, it gives you the ability to 
> return a foreign-syntax path as the "lldbinit" location. Therefore, in the 
> spirit of using the most specific type which is sufficient to accomplish a 
> given task, I think we should use a plain string here.

I'm especially concerned about windows support, so I also agree with this.




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2149-2150
+
+  LoadCWDlldbinitFile should_load = ShouldLoadCwdInitFile();
+  if (should_load == eLoadCWDlldbinitFalse) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);

labath wrote:
> Make this a `switch` ?
+1



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

labath wrote:
> JDevlieghere wrote:
> > This should be reflowed. 
> What might help readability is moving the long block of text to a separate 
> static variable declared before the function. That way the text does not 
> obscure the logic of the function.
I agree with Pavel, that would indeed make things more readable.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think this is a bit more readable. I've included some suggestions which I 
think could make it even better.

Since you're already rewriting this code, this might be a good time to raise a 
point I wanted to bring up some day. Should we be using `FileSpec` for code 
like this? The code already uses a combination of llvm and lldb path utilities, 
and I would argue that we should use llvm all the way for code like this 
(except that we go through the FileSystem abstraction for the reproducer 
stuff). I have two reasons for that:

- FileSpec is designed for efficient matching of abstract file paths which may 
not even exist on the host system. As such, this code will result in a bunch of 
strings being added to the global string pool for no reason. And in this case, 
you're definitely working files which do exist (or may exist) on the host. In 
fact, FileSpec now contains code which performs path simplifications which are 
not completely sound given a concrete filesystem -- it will simplify a path 
like `bar/../foo` to `foo`, which is not correct if `bar` is a symlink.
- Since we started supporting windows paths, the FileSpec class offers more 
freedom than it is needed here. Specifically, it gives you the ability to 
return a foreign-syntax path as the "lldbinit" location. Therefore, in the 
spirit of using the most specific type which is sufficient to accomplish a 
given task, I think we should use a plain string here.




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2149-2150
+
+  LoadCWDlldbinitFile should_load = ShouldLoadCwdInitFile();
+  if (should_load == eLoadCWDlldbinitFalse) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);

Make this a `switch` ?



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

JDevlieghere wrote:
> This should be reflowed. 
What might help readability is moving the long block of text to a separate 
static variable declared before the function. That way the text does not 
obscure the logic of the function.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2191-2196
+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.GetPath().c_str(), program_name);

This should be done with strings and stringrefs


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2118
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject &result) {
+  if (!FileSystem::Instance().Exists(file)) {

This should also have an `assert(!m_skip_lldbinit_files);`



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

This should be reflowed. 


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61994/new/

https://reviews.llvm.org/D61994



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61994: [CommandInterpreter] Refactor SourceInitFile

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, xiaobai, jingham.
Herald added a project: LLDB.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2118
+void CommandInterpreter::SourceInitFile(FileSpec file,
 CommandReturnObject &result) {
+  if (!FileSystem::Instance().Exists(file)) {

This should also have an `assert(!m_skip_lldbinit_files);`



Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2163
+result.AppendErrorWithFormat(
+"There is a .lldbinit file in the current directory which is not "
+"being read.\n"

This should be reflowed. 


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`. Furthermore 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). Finally, there were at 
least two branches where the status of the CommandReturnObject were not set.

Anyway, this is an attempt to simplify this code a bit. It's still more complex 
than I had hoped.

Please let me know if you think it's more readable or not.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D61994

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Interpreter/CommandInterpreter.cpp

Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2091,100 +2091,130 @@
   return position;
 }
 
-void CommandInterpreter::SourceInitFile(bool in_cwd,
+static FileSpec GetHomeInitFile(llvm::StringRef suffix = {}) {
+  llvm::SmallString<64> buffer;
+  llvm::sys::path::home_directory(buffer);
+  llvm::sys::path::append(buffer, ".lldbinit");
+
+  FileSpec init_file(buffer.c_str());
+  FileSystem::Instance().Resolve(init_file);
+  return init_file;
+}
+
+static FileSpec GetCwdInitFile() {
+  FileSpec init_file(".lldbinit");
+  FileSystem::Instance().Resolve(init_file);
+  return 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;
+  if (!FileSystem::Instance().Exists(file)) {
+result.SetStatus(eReturnStatusSuccessFinishNoResult);
+return;
+  }
 
-  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);
-  }
-