Optimizations for FileSpec.

llvm-svn: 215124

5 files changed, 108 insertions, 172 deletions

diff --git a/lldb/source/API/SBFileSpec.cpp b/lldb/source/API/SBFileSpec.cpp
index 0de1c245846..8d63fc587d8 100644
--- a/lldb/source/API/SBFileSpec.cpp
+++ b/lldb/source/API/SBFileSpec.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Core/Log.h"
 #include "lldb/Core/Stream.h"
 
+#include "llvm/ADT/SmallString.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -89,7 +91,11 @@ SBFileSpec::ResolveExecutableLocation ()
 int
 SBFileSpec::ResolvePath (const char *src_path, char *dst_path, size_t dst_len)
 {
-    return lldb_private::FileSpec::Resolve (src_path, dst_path, dst_len);
+    llvm::SmallString<64> result(src_path);
+    lldb_private::FileSpec::Resolve (result);
+    size_t result_length = std::min(dst_len-1, result.size());
+    ::strncpy(dst_path, result.c_str(), result_length + 1);
+    return result_length;
 }
 
 const char *
diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp
index 970aa692bd2..f0ad4a89673 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -30,6 +30,8 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/CleanUp.h"
 
+#include "llvm/ADT/SmallString.h"
+
 using namespace lldb_private;
 
 CommandCompletions::CommonCompletionElement
@@ -221,7 +223,7 @@ DiskFilesOrDirectories
     end_ptr = strrchr(partial_name_copy, '/');
     
     // This will store the resolved form of the containing directory
-    char containing_part[PATH_MAX];
+    llvm::SmallString<64> containing_part;
     
     if (end_ptr == NULL)
     {
@@ -232,14 +234,11 @@ DiskFilesOrDirectories
             // Nothing here but the user name.  We could just put a slash on the end, 
             // but for completeness sake we'll resolve the user name and only put a slash
             // on the end if it exists.
-            char resolved_username[PATH_MAX];
-            size_t resolved_username_len = FileSpec::ResolveUsername (partial_name_copy, resolved_username, 
-                                                          sizeof (resolved_username));
+            llvm::SmallString<64> resolved_username(partial_name_copy);
+            FileSpec::ResolveUsername (resolved_username);
                                                           
            // Not sure how this would happen, a username longer than PATH_MAX?  Still...
-            if (resolved_username_len >= sizeof (resolved_username))
-                return matches.GetSize();
-            else if (resolved_username_len == 0)
+            if (resolved_username.size() == 0)
             {
                 // The user name didn't resolve, let's look in the password database for matches.
                 // The user name database contains duplicates, and is not in alphabetical order, so
@@ -263,8 +262,7 @@ DiskFilesOrDirectories
         else
         {
             // The containing part is the CWD, and the whole string is the remainder.
-            containing_part[0] = '.';
-            containing_part[1] = '\0';
+            containing_part = ".";
             strcpy(remainder, partial_name_copy);
             end_ptr = partial_name_copy;
         }
@@ -274,14 +272,11 @@ DiskFilesOrDirectories
         if (end_ptr == partial_name_copy)
         {
             // We're completing a file or directory in the root volume.
-            containing_part[0] = '/';
-            containing_part[1] = '\0';
+            containing_part = "/";
         }
         else
         {
-            size_t len = end_ptr - partial_name_copy;
-            memcpy(containing_part, partial_name_copy, len);
-            containing_part[len] = '\0';
+            containing_part.append(partial_name_copy, end_ptr);
         }
         // Push end_ptr past the final "/" and set remainder.
         end_ptr++;
@@ -293,11 +288,9 @@ DiskFilesOrDirectories
 
     if (*partial_name_copy == '~')
     {
-        size_t resolved_username_len = FileSpec::ResolveUsername(containing_part, 
-                                                                 containing_part, 
-                                                                 sizeof (containing_part));
+        FileSpec::ResolveUsername(containing_part);
         // User name doesn't exist, we're not getting any further...
-        if (resolved_username_len == 0 || resolved_username_len >= sizeof (containing_part))
+        if (containing_part.empty())
             return matches.GetSize();
     }
 
@@ -314,7 +307,7 @@ DiskFilesOrDirectories
     parameters.end_ptr = end_ptr;
     parameters.baselen = baselen;
 
-    FileSpec::EnumerateDirectory(containing_part, true, true, true, DiskFilesOrDirectoriesCallback, &parameters);
+    FileSpec::EnumerateDirectory(containing_part.c_str(), true, true, true, DiskFilesOrDirectoriesCallback, &parameters);
     
     return matches.GetSize();
 }
diff --git a/lldb/source/Host/common/FileSpec.cpp b/lldb/source/Host/common/FileSpec.cpp
index 845a3183001..615772e08bd 100644
--- a/lldb/source/Host/common/FileSpec.cpp
+++ b/lldb/source/Host/common/FileSpec.cpp
@@ -28,6 +28,7 @@
 #endif
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 
@@ -77,81 +78,66 @@ GetCachedGlobTildeSlash()
 
 // Resolves the username part of a path of the form ~user/other/directories, and
 // writes the result into dst_path.
-// Returns 0 if there WAS a ~ in the path but the username couldn't be resolved.
-// Otherwise returns the number of characters copied into dst_path.  If the return
-// is >= dst_len, then the resolved path is too long...
-size_t
-FileSpec::ResolveUsername (const char *src_path, char *dst_path, size_t dst_len)
+void
+FileSpec::ResolveUsername (llvm::SmallVectorImpl<char> &path)
 {
-    if (src_path == NULL || src_path[0] == '\0')
-        return 0;
-
-#ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
-
-    char user_home[PATH_MAX];
-    const char *user_name;
-    
-    
-    // If there's no ~, then just copy src_path straight to dst_path (they may be the same string...)
-    if (src_path[0] != '~')
-    {
-        size_t len = strlen (src_path);
-        if (len >= dst_len)
-        {
-            ::bcopy (src_path, dst_path, dst_len - 1);
-            dst_path[dst_len] = '\0';
-        }
-        else
-            ::bcopy (src_path, dst_path, len + 1);
-        
-        return len;
-    }
-    
-    const char *first_slash = ::strchr (src_path, '/');
-    char remainder[PATH_MAX];
+#if LLDB_CONFIG_TILDE_RESOLVES_TO_USER
+    if (path.empty() || path[0] != '~')
+        return;
     
-    if (first_slash == NULL)
+    llvm::StringRef path_str(path.data());
+    size_t slash_pos = path_str.find_first_of("/", 1);
+    if (slash_pos == 1)
     {
-        // The whole name is the username (minus the ~):
-        user_name = src_path + 1;
-        remainder[0] = '\0';
-    }
-    else
-    {
-        size_t user_name_len = first_slash - src_path - 1;
-        ::memcpy (user_home, src_path + 1, user_name_len);
-        user_home[user_name_len] = '\0';
-        user_name = user_home;
+        // A path of the form ~/ resolves to the current user's home dir
+        llvm::SmallString<64> home_dir;
+        if (!llvm::sys::path::home_directory(home_dir))
+            return;
         
-        ::strcpy (remainder, first_slash);
+        // Overwrite the ~ with the first character of the homedir, and insert
+        // the rest.  This way we only trigger one move, whereas an insert
+        // followed by a delete (or vice versa) would trigger two.
+        path[0] = home_dir[0];
+        path.insert(path.begin() + 1, home_dir.begin() + 1, home_dir.end());
+        return;
     }
     
-    if (user_name == NULL)
-        return 0;
-    // User name of "" means the current user...
-    
-    struct passwd *user_entry;
-    const char *home_dir = NULL;
-    
-    if (user_name[0] == '\0')
+    auto username_begin = path.begin()+1;
+    auto username_end = (slash_pos == llvm::StringRef::npos)
+                        ? path.end()
+                        : (path.begin() + slash_pos);
+    size_t replacement_length = std::distance(path.begin(), username_end);
+
+    llvm::SmallString<20> username(username_begin, username_end);
+    struct passwd *user_entry = ::getpwnam(username.c_str());
+    if (user_entry != nullptr)
     {
-        home_dir = GetCachedGlobTildeSlash();
+        // Copy over the first n characters of the path, where n is the smaller of the length
+        // of the home directory and the slash pos.
+        llvm::StringRef homedir(user_entry->pw_dir);
+        size_t initial_copy_length = std::min(homedir.size(), replacement_length);
+        auto src_begin = homedir.begin();
+        auto src_end = src_begin + initial_copy_length;
+        std::copy(src_begin, src_end, path.begin());
+        if (replacement_length > homedir.size())
+        {
+            // We copied the entire home directory, but the ~username portion of the path was
+            // longer, so there's characters that need to be removed.
+            path.erase(path.begin() + initial_copy_length, username_end);
+        }
+        else if (replacement_length < homedir.size())
+        {
+            // We copied all the way up to the slash in the destination, but there's still more
+            // characters that need to be inserted.
+            path.insert(username_end, src_end, homedir.end());
+        }
     }
     else
     {
-        user_entry = ::getpwnam (user_name);
-        if (user_entry != NULL)
-            home_dir = user_entry->pw_dir;
+        // Unable to resolve username (user doesn't exist?)
+        path.clear();
     }
-    
-    if (home_dir == NULL)
-        return 0;
-    else 
-        return ::snprintf (dst_path, dst_len, "%s%s", home_dir, remainder);
-#else
-    // Resolving home directories is not supported, just copy the path...
-    return ::snprintf (dst_path, dst_len, "%s", src_path);
-#endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER    
+#endif
 }
 
 size_t
@@ -187,44 +173,18 @@ FileSpec::ResolvePartialUsername (const char *partial_name, StringList &matches)
 #endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER    
 }
 
-
-
-size_t
-FileSpec::Resolve (const char *src_path, char *dst_path, size_t dst_len)
+void
+FileSpec::Resolve (llvm::SmallVectorImpl<char> &path)
 {
-    if (src_path == NULL || src_path[0] == '\0')
-        return 0;
+    if (path.empty())
+        return;
 
-    // Glob if needed for ~/, otherwise copy in case src_path is same as dst_path...
-    char unglobbed_path[PATH_MAX];
 #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
-    if (src_path[0] == '~')
-    {
-        size_t return_count = ResolveUsername(src_path, unglobbed_path, sizeof(unglobbed_path));
-        
-        // If we couldn't find the user referred to, or the resultant path was too long,
-        // then just copy over the src_path.
-        if (return_count == 0 || return_count >= sizeof(unglobbed_path)) 
-            ::snprintf (unglobbed_path, sizeof(unglobbed_path), "%s", src_path);
-    }
-    else
+    if (path[0] == '~')
+        ResolveUsername(path);
 #endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
-    {
-    	::snprintf(unglobbed_path, sizeof(unglobbed_path), "%s", src_path);
-    }
 
-    // Now resolve the path if needed
-    char resolved_path[PATH_MAX];
-    if (::realpath (unglobbed_path, resolved_path))
-    {
-        // Success, copy the resolved path
-        return ::snprintf(dst_path, dst_len, "%s", resolved_path);
-    }
-    else
-    {
-        // Failed, just copy the unglobbed path
-        return ::snprintf(dst_path, dst_len, "%s", unglobbed_path);
-    }
+    llvm::sys::fs::make_absolute(path);
 }
 
 FileSpec::FileSpec() :
@@ -292,24 +252,20 @@ FileSpec::operator= (const FileSpec& rhs)
     return *this;
 }
 
-void FileSpec::Normalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax)
+void FileSpec::Normalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax)
 {
-    result.clear();
-    result.append(path.begin(), path.end());
     if (syntax == ePathSyntaxPosix || (syntax == ePathSyntaxHostNative && Host::GetHostPathSyntax() == ePathSyntaxPosix))
         return;
 
-    std::replace(result.begin(), result.end(), '\\', '/');
+    std::replace(path.begin(), path.end(), '\\', '/');
 }
 
-void FileSpec::DeNormalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax)
+void FileSpec::DeNormalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax)
 {
-    result.clear();
-    result.append(path.begin(), path.end());
     if (syntax == ePathSyntaxPosix || (syntax == ePathSyntaxHostNative && Host::GetHostPathSyntax() == ePathSyntaxPosix))
         return;
 
-    std::replace(result.begin(), result.end(), '/', '\\');
+    std::replace(path.begin(), path.end(), '/', '\\');
 }
 
 //------------------------------------------------------------------
@@ -328,41 +284,26 @@ FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax)
     if (pathname == NULL || pathname[0] == '\0')
         return;
 
-    llvm::SmallString<64> normalized;
-    Normalize(pathname, normalized, syntax);
+    llvm::SmallString<64> normalized(pathname);
+    Normalize(normalized, syntax);
 
-    std::vector<char> resolved_path(PATH_MAX);
-    bool path_fit = true;
-    
     if (resolve)
     {
-        path_fit = (FileSpec::Resolve (normalized.c_str(), &resolved_path[0], resolved_path.size()) < resolved_path.size() - 1);
-        m_is_resolved = path_fit;
-    }
-    else
-    {
-        // Copy the path because "basename" and "dirname" want to muck with the
-        // path buffer
-        if (normalized.size() > resolved_path.size() - 1)
-            path_fit = false;
-        else
-            ::strcpy (&resolved_path[0], normalized.c_str());
+        FileSpec::Resolve (normalized);
+        m_is_resolved = true;
     }
     
-    if (path_fit)
+    llvm::StringRef resolve_path_ref(normalized.c_str());
+    llvm::StringRef filename_ref = llvm::sys::path::filename(resolve_path_ref);
+    if (!filename_ref.empty())
     {
-        llvm::StringRef resolve_path_ref(&resolved_path[0]);
-        llvm::StringRef filename_ref = llvm::sys::path::filename(resolve_path_ref);
-        if (!filename_ref.empty())
-        {
-            m_filename.SetString (filename_ref);
-            llvm::StringRef directory_ref = llvm::sys::path::parent_path(resolve_path_ref);
-            if (!directory_ref.empty())
-                m_directory.SetString(directory_ref);
-        }
-        else
-            m_directory.SetCString(&resolved_path[0]);
+        m_filename.SetString (filename_ref);
+        llvm::StringRef directory_ref = llvm::sys::path::parent_path(resolve_path_ref);
+        if (!directory_ref.empty())
+            m_directory.SetString(directory_ref);
     }
+    else
+        m_directory.SetCString(normalized.c_str());
 }
 
 //----------------------------------------------------------------------
@@ -732,11 +673,7 @@ FileSpec::GetPath (bool denormalize) const
     if (m_filename)
         llvm::sys::path::append(result, m_filename.GetCString());
     if (denormalize && !result.empty())
-    {
-        llvm::SmallString<64> denormalized;
-        DeNormalize(result.c_str(), denormalized, m_syntax);
-        result = denormalized;
-    }
+        DeNormalize(result, m_syntax);
 
     return std::string(result.begin(), result.end());
 }
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index a19608365ec..c469ead2be6 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -1119,7 +1119,6 @@ Host::GetLLDBPath (PathType path_type, FileSpec &file_spec)
                 if (GetLLDBPath (ePathTypeLLDBShlibDir, lldb_file_spec))
                 {
                     char raw_path[PATH_MAX];
-                    char resolved_path[PATH_MAX];
                     lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
 
 #if defined (__APPLE__)
@@ -1159,8 +1158,9 @@ Host::GetLLDBPath (PathType path_type, FileSpec &file_spec)
                             log->Printf ("Host::%s() failed to find /lib/liblldb within the shared lib path, bailing on bin path construction", __FUNCTION__);
                     }
 #endif  // #if defined (__APPLE__)
-                    FileSpec::Resolve (raw_path, resolved_path, sizeof(resolved_path));
-                    g_lldb_support_exe_dir.SetCString(resolved_path);
+                    llvm::SmallString<64> resolved_path(raw_path);
+                    FileSpec::Resolve (resolved_path);
+                    g_lldb_support_exe_dir.SetCString(resolved_path.c_str());
                 }
                 if (log)
                     log->Printf("Host::GetLLDBPath(ePathTypeSupportExecutableDir) => '%s'", g_lldb_support_exe_dir.GetCString());
@@ -1217,7 +1217,6 @@ Host::GetLLDBPath (PathType path_type, FileSpec &file_spec)
                 if (GetLLDBPath (ePathTypeLLDBShlibDir, lldb_file_spec))
                 {
                     char raw_path[PATH_MAX];
-                    char resolved_path[PATH_MAX];
 #if defined(_WIN32)
                     lldb_file_spec.AppendPathComponent("../lib/site-packages");
                     lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
@@ -1248,8 +1247,9 @@ Host::GetLLDBPath (PathType path_type, FileSpec &file_spec)
 #if defined (__APPLE__)
                     }
 #endif
-                    FileSpec::Resolve (raw_path, resolved_path, sizeof(resolved_path));
-                    g_lldb_python_dir.SetCString(resolved_path);
+                    llvm::SmallString<64> resolved_path(raw_path);
+                    FileSpec::Resolve (resolved_path);
+                    g_lldb_python_dir.SetCString(resolved_path.c_str());
                 }
                 
                 if (log)
diff --git a/lldb/source/Target/TargetList.cpp b/lldb/source/Target/TargetList.cpp
index 091211d9cb8..d1775213d60 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -28,6 +28,8 @@
 #include "lldb/Target/Process.h"
 #include "lldb/Target/TargetList.h"
 
+#include "llvm/ADT/SmallString.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -241,15 +243,13 @@ TargetList::CreateTarget (Debugger &debugger,
     {
         // we want to expand the tilde but we don't want to resolve any symbolic links
         // so we can't use the FileSpec constructor's resolve flag
-        char unglobbed_path[PATH_MAX];
-        unglobbed_path[0] = '\0';
-
-        size_t return_count = FileSpec::ResolveUsername(user_exe_path, unglobbed_path, sizeof(unglobbed_path));
-
-        if (return_count == 0 || return_count >= sizeof(unglobbed_path))
-            ::snprintf (unglobbed_path, sizeof(unglobbed_path), "%s", user_exe_path);
+        llvm::SmallString<64> unglobbed_path(user_exe_path);
+        FileSpec::ResolveUsername(unglobbed_path);
 
-        file = FileSpec(unglobbed_path, false);
+        if (unglobbed_path.empty())
+            file = FileSpec(user_exe_path, false);
+        else
+            file = FileSpec(unglobbed_path.c_str(), false);
     }
 
     bool user_exe_path_is_bundle = false;