[Lldb-commits] [lldb] r216398 - Change back all paths returns for lldb::PathType enumerations from HostInfo::GetLLDBPath() to return the directories in the FileSpec.m_directory field to match previous implementations. This change previously broke some path stuff in upstream branches.

Zachary Turner zturner at google.com
Mon Aug 25 19:05:04 PDT 2014


It's possible I'm misunderstanding how this class works, or is supposed to
work.  Either way, its interface is confusing as written, so perhaps it
needs to be re-thought.

But, if you call FileSpec::SetFile(foo), then my understanding is that foo
represents an arbitrary path.  It might be a directory, it might be a file,
doesn't matter.  FileSpec internally stores everything except the last
component as the "directory", and the last component as the "file", even if
directory+file actually points to a directory.

Is this not how it's supposed to work?  I think it's confusing and
inconsistent if that's not how it works.  In other words, a multi-component
"directory" field and a null path should be an invalid FileSpec in my mind.
 If you want to get back the entire thing, don't use
file_spec.GetDirectory(), just use file_spec.GetPath(), that's what it's
for.

Does this make sense?  Thoughts?


On Mon, Aug 25, 2014 at 11:21 AM, Greg Clayton <gclayton at apple.com> wrote:

> Author: gclayton
> Date: Mon Aug 25 13:21:06 2014
> New Revision: 216398
>
> URL: http://llvm.org/viewvc/llvm-project?rev=216398&view=rev
> Log:
> Change back all paths returns for lldb::PathType enumerations from
> HostInfo::GetLLDBPath() to return the directories in the
> FileSpec.m_directory field to match previous implementations. This change
> previously broke some path stuff in upstream branches.
>
>
> Modified:
>     lldb/trunk/source/Host/common/HostInfoBase.cpp
>     lldb/trunk/source/Host/linux/HostInfoLinux.cpp
>     lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
>     lldb/trunk/source/Host/posix/HostInfoPosix.cpp
>     lldb/trunk/source/Host/windows/HostInfoWindows.cpp
>
> Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=216398&r1=216397&r2=216398&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Host/common/HostInfoBase.cpp (original)
> +++ lldb/trunk/source/Host/common/HostInfoBase.cpp Mon Aug 25 13:21:06 2014
> @@ -228,7 +228,7 @@ HostInfoBase::ComputeSharedLibraryDirect
>          Host::GetModuleFileSpecForHostAddress(reinterpret_cast<void
> *>(reinterpret_cast<intptr_t>(HostInfoBase::GetLLDBPath))));
>
>      // Remove the filename so that this FileSpec only represents the
> directory.
> -    file_spec.SetFile(lldb_file_spec.GetDirectory().AsCString(), true);
> +    file_spec.GetDirectory() = lldb_file_spec.GetDirectory();
>
>      return (bool)file_spec.GetDirectory();
>  }
> @@ -264,7 +264,7 @@ HostInfoBase::ComputeTempFileDirectory(F
>      // Make an atexit handler to clean up the process specify LLDB temp
> dir
>      // and all of its contents.
>      ::atexit(CleanupProcessSpecificLLDBTempDir);
> -    file_spec.SetFile(pid_tmpdir.GetString().c_str(), false);
> +
> file_spec.GetDirectory().SetCStringWithLength(pid_tmpdir.GetString().c_str(),
> pid_tmpdir.GetString().size());
>      return true;
>  }
>
>
> Modified: lldb/trunk/source/Host/linux/HostInfoLinux.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/HostInfoLinux.cpp?rev=216398&r1=216397&r2=216398&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Host/linux/HostInfoLinux.cpp (original)
> +++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp Mon Aug 25 13:21:06 2014
> @@ -194,7 +194,8 @@ HostInfoLinux::GetProgramFileSpec()
>  bool
>  HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec)
>  {
> -    file_spec.SetFile("/usr/lib/lldb", true);
> +    FileSpec temp_file("/usr/lib/lldb", true);
> +    file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
>      return true;
>  }
>
> @@ -204,17 +205,15 @@ HostInfoLinux::ComputeUserPluginsDirecto
>      // XDG Base Directory Specification
>      //
> http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
>      // If XDG_DATA_HOME exists, use that, otherwise use
> ~/.local/share/lldb.
> -    FileSpec lldb_file_spec;
>      const char *xdg_data_home = getenv("XDG_DATA_HOME");
>      if (xdg_data_home && xdg_data_home[0])
>      {
>          std::string user_plugin_dir(xdg_data_home);
>          user_plugin_dir += "/lldb";
> -        lldb_file_spec.SetFile(user_plugin_dir.c_str(), true);
> +        file_spec.GetDirectory().SetCString(user_plugin_dir.c_str());
>      }
>      else
> -        lldb_file_spec.SetFile("~/.local/share/lldb", true);
> -
> +        file_spec.GetDirectory().SetCString("~/.local/share/lldb");
>      return true;
>  }
>
>
> Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=216398&r1=216397&r2=216398&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm (original)
> +++ lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Mon Aug 25 13:21:06
> 2014
> @@ -149,7 +149,7 @@ HostInfoMacOSX::ComputeSupportExeDirecto
>          ::strncpy(framework_pos, "/Resources", PATH_MAX - (framework_pos
> - raw_path));
>  #endif
>      }
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return (bool)file_spec.GetDirectory();
>  }
>
> @@ -169,7 +169,7 @@ HostInfoMacOSX::ComputeHeaderDirectory(F
>          framework_pos += strlen("LLDB.framework");
>          ::strncpy(framework_pos, "/Headers", PATH_MAX - (framework_pos -
> raw_path));
>      }
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return true;
>  }
>
> @@ -199,7 +199,7 @@ HostInfoMacOSX::ComputePythonDirectory(F
>          // We may get our string truncated. Should we protect this with
> an assert?
>          ::strncat(raw_path, python_version_dir.c_str(), sizeof(raw_path)
> - strlen(raw_path) - 1);
>      }
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return true;
>  }
>
> @@ -218,14 +218,15 @@ HostInfoMacOSX::ComputeSystemPluginsDire
>
>      framework_pos += strlen("LLDB.framework");
>      ::strncpy(framework_pos, "/Resources/PlugIns", PATH_MAX -
> (framework_pos - raw_path));
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return true;
>  }
>
>  bool
>  HostInfoMacOSX::ComputeUserPluginsDirectory(FileSpec &file_spec)
>  {
> -    file_spec.SetFile("~/Library/Application Support/LLDB/PlugIns", true);
> +    FileSpec temp_file("~/Library/Application Support/LLDB/PlugIns",
> true);
> +    file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
>      return true;
>  }
>
>
> Modified: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=216398&r1=216397&r2=216398&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Host/posix/HostInfoPosix.cpp (original)
> +++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp Mon Aug 25 13:21:06 2014
> @@ -158,14 +158,15 @@ HostInfoPosix::ComputeSupportExeDirector
>              log->Printf("Host::%s() failed to find /lib/liblldb within
> the shared lib path, bailing on bin path construction",
>                          __FUNCTION__);
>      }
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return (bool)file_spec.GetDirectory();
>  }
>
>  bool
>  HostInfoPosix::ComputeHeaderDirectory(FileSpec &file_spec)
>  {
> -    file_spec.SetFile("/opt/local/include/lldb", false);
> +    FileSpec temp_file("/opt/local/include/lldb", false);
> +    file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());
>      return true;
>  }
>
> @@ -187,6 +188,6 @@ HostInfoPosix::ComputePythonDirectory(Fi
>      // We may get our string truncated. Should we protect this with an
> assert?
>      ::strncat(raw_path, python_version_dir.c_str(), sizeof(raw_path) -
> strlen(raw_path) - 1);
>
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return true;
>  }
>
> Modified: lldb/trunk/source/Host/windows/HostInfoWindows.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=216398&r1=216397&r2=216398&view=diff
>
> ==============================================================================
> --- lldb/trunk/source/Host/windows/HostInfoWindows.cpp (original)
> +++ lldb/trunk/source/Host/windows/HostInfoWindows.cpp Mon Aug 25 13:21:06
> 2014
> @@ -107,6 +107,6 @@ HostInfoWindows::ComputePythonDirectory(
>      lldb_file_spec.AppendPathComponent("../lib/site-packages");
>      lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
>
> -    file_spec.SetFile(raw_path, true);
> +    file_spec.GetDirectory().SetCString(raw_path);
>      return true;
>  }
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140825/d38ac0e0/attachment.html>


More information about the lldb-commits mailing list