<div dir="ltr">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.<div><br></div><div>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.</div>
<div><br></div><div>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.</div>
<div><br></div><div>Does this make sense? Thoughts?</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 25, 2014 at 11:21 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: gclayton<br>
Date: Mon Aug 25 13:21:06 2014<br>
New Revision: 216398<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=216398&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=216398&view=rev</a><br>
Log:<br>
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.<br>
<br>
<br>
Modified:<br>
lldb/trunk/source/Host/common/HostInfoBase.cpp<br>
lldb/trunk/source/Host/linux/HostInfoLinux.cpp<br>
lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm<br>
lldb/trunk/source/Host/posix/HostInfoPosix.cpp<br>
lldb/trunk/source/Host/windows/HostInfoWindows.cpp<br>
<br>
Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=216398&r1=216397&r2=216398&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=216398&r1=216397&r2=216398&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/common/HostInfoBase.cpp (original)<br>
+++ lldb/trunk/source/Host/common/HostInfoBase.cpp Mon Aug 25 13:21:06 2014<br>
@@ -228,7 +228,7 @@ HostInfoBase::ComputeSharedLibraryDirect<br>
Host::GetModuleFileSpecForHostAddress(reinterpret_cast<void *>(reinterpret_cast<intptr_t>(HostInfoBase::GetLLDBPath))));<br>
<br>
// Remove the filename so that this FileSpec only represents the directory.<br>
- file_spec.SetFile(lldb_file_spec.GetDirectory().AsCString(), true);<br>
+ file_spec.GetDirectory() = lldb_file_spec.GetDirectory();<br>
<br>
return (bool)file_spec.GetDirectory();<br>
}<br>
@@ -264,7 +264,7 @@ HostInfoBase::ComputeTempFileDirectory(F<br>
// Make an atexit handler to clean up the process specify LLDB temp dir<br>
// and all of its contents.<br>
::atexit(CleanupProcessSpecificLLDBTempDir);<br>
- file_spec.SetFile(pid_tmpdir.GetString().c_str(), false);<br>
+ file_spec.GetDirectory().SetCStringWithLength(pid_tmpdir.GetString().c_str(), pid_tmpdir.GetString().size());<br>
return true;<br>
}<br>
<br>
<br>
Modified: lldb/trunk/source/Host/linux/HostInfoLinux.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/HostInfoLinux.cpp?rev=216398&r1=216397&r2=216398&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/HostInfoLinux.cpp?rev=216398&r1=216397&r2=216398&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/linux/HostInfoLinux.cpp (original)<br>
+++ lldb/trunk/source/Host/linux/HostInfoLinux.cpp Mon Aug 25 13:21:06 2014<br>
@@ -194,7 +194,8 @@ HostInfoLinux::GetProgramFileSpec()<br>
bool<br>
HostInfoLinux::ComputeSystemPluginsDirectory(FileSpec &file_spec)<br>
{<br>
- file_spec.SetFile("/usr/lib/lldb", true);<br>
+ FileSpec temp_file("/usr/lib/lldb", true);<br>
+ file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());<br>
return true;<br>
}<br>
<br>
@@ -204,17 +205,15 @@ HostInfoLinux::ComputeUserPluginsDirecto<br>
// XDG Base Directory Specification<br>
// <a href="http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html" target="_blank">http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html</a><br>
// If XDG_DATA_HOME exists, use that, otherwise use ~/.local/share/lldb.<br>
- FileSpec lldb_file_spec;<br>
const char *xdg_data_home = getenv("XDG_DATA_HOME");<br>
if (xdg_data_home && xdg_data_home[0])<br>
{<br>
std::string user_plugin_dir(xdg_data_home);<br>
user_plugin_dir += "/lldb";<br>
- lldb_file_spec.SetFile(user_plugin_dir.c_str(), true);<br>
+ file_spec.GetDirectory().SetCString(user_plugin_dir.c_str());<br>
}<br>
else<br>
- lldb_file_spec.SetFile("~/.local/share/lldb", true);<br>
-<br>
+ file_spec.GetDirectory().SetCString("~/.local/share/lldb");<br>
return true;<br>
}<br>
<br>
<br>
Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=216398&r1=216397&r2=216398&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=216398&r1=216397&r2=216398&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm (original)<br>
+++ lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Mon Aug 25 13:21:06 2014<br>
@@ -149,7 +149,7 @@ HostInfoMacOSX::ComputeSupportExeDirecto<br>
::strncpy(framework_pos, "/Resources", PATH_MAX - (framework_pos - raw_path));<br>
#endif<br>
}<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return (bool)file_spec.GetDirectory();<br>
}<br>
<br>
@@ -169,7 +169,7 @@ HostInfoMacOSX::ComputeHeaderDirectory(F<br>
framework_pos += strlen("LLDB.framework");<br>
::strncpy(framework_pos, "/Headers", PATH_MAX - (framework_pos - raw_path));<br>
}<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return true;<br>
}<br>
<br>
@@ -199,7 +199,7 @@ HostInfoMacOSX::ComputePythonDirectory(F<br>
// We may get our string truncated. Should we protect this with an assert?<br>
::strncat(raw_path, python_version_dir.c_str(), sizeof(raw_path) - strlen(raw_path) - 1);<br>
}<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return true;<br>
}<br>
<br>
@@ -218,14 +218,15 @@ HostInfoMacOSX::ComputeSystemPluginsDire<br>
<br>
framework_pos += strlen("LLDB.framework");<br>
::strncpy(framework_pos, "/Resources/PlugIns", PATH_MAX - (framework_pos - raw_path));<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return true;<br>
}<br>
<br>
bool<br>
HostInfoMacOSX::ComputeUserPluginsDirectory(FileSpec &file_spec)<br>
{<br>
- file_spec.SetFile("~/Library/Application Support/LLDB/PlugIns", true);<br>
+ FileSpec temp_file("~/Library/Application Support/LLDB/PlugIns", true);<br>
+ file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());<br>
return true;<br>
}<br>
<br>
<br>
Modified: lldb/trunk/source/Host/posix/HostInfoPosix.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=216398&r1=216397&r2=216398&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=216398&r1=216397&r2=216398&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp (original)<br>
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp Mon Aug 25 13:21:06 2014<br>
@@ -158,14 +158,15 @@ HostInfoPosix::ComputeSupportExeDirector<br>
log->Printf("Host::%s() failed to find /lib/liblldb within the shared lib path, bailing on bin path construction",<br>
__FUNCTION__);<br>
}<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return (bool)file_spec.GetDirectory();<br>
}<br>
<br>
bool<br>
HostInfoPosix::ComputeHeaderDirectory(FileSpec &file_spec)<br>
{<br>
- file_spec.SetFile("/opt/local/include/lldb", false);<br>
+ FileSpec temp_file("/opt/local/include/lldb", false);<br>
+ file_spec.GetDirectory().SetCString(temp_file.GetPath().c_str());<br>
return true;<br>
}<br>
<br>
@@ -187,6 +188,6 @@ HostInfoPosix::ComputePythonDirectory(Fi<br>
// We may get our string truncated. Should we protect this with an assert?<br>
::strncat(raw_path, python_version_dir.c_str(), sizeof(raw_path) - strlen(raw_path) - 1);<br>
<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return true;<br>
}<br>
<br>
Modified: lldb/trunk/source/Host/windows/HostInfoWindows.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=216398&r1=216397&r2=216398&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/HostInfoWindows.cpp?rev=216398&r1=216397&r2=216398&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Host/windows/HostInfoWindows.cpp (original)<br>
+++ lldb/trunk/source/Host/windows/HostInfoWindows.cpp Mon Aug 25 13:21:06 2014<br>
@@ -107,6 +107,6 @@ HostInfoWindows::ComputePythonDirectory(<br>
lldb_file_spec.AppendPathComponent("../lib/site-packages");<br>
lldb_file_spec.GetPath(raw_path, sizeof(raw_path));<br>
<br>
- file_spec.SetFile(raw_path, true);<br>
+ file_spec.GetDirectory().SetCString(raw_path);<br>
return true;<br>
}<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@cs.uiuc.edu">lldb-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</blockquote></div><br></div>