[Lldb-commits] [lldb] r215124 - Optimizations for FileSpec.
Zachary Turner
zturner at google.com
Thu Aug 7 10:33:36 PDT 2014
Author: zturner
Date: Thu Aug 7 12:33:36 2014
New Revision: 215124
URL: http://llvm.org/viewvc/llvm-project?rev=215124&view=rev
Log:
Optimizations for FileSpec.
Modified:
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/source/API/SBFileSpec.cpp
lldb/trunk/source/Commands/CommandCompletions.cpp
lldb/trunk/source/Host/common/FileSpec.cpp
lldb/trunk/source/Host/common/Host.cpp
lldb/trunk/source/Target/TargetList.cpp
Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Thu Aug 7 12:33:36 2014
@@ -570,8 +570,8 @@ public:
lldb::DataBufferSP
ReadFileContentsAsCString(Error *error_ptr = NULL);
- static void Normalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax);
- static void DeNormalize(llvm::StringRef path, llvm::SmallVectorImpl<char> &result, PathSyntax syntax);
+ static void Normalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
+ static void DeNormalize(llvm::SmallVectorImpl<char> &path, PathSyntax syntax = ePathSyntaxHostNative);
//------------------------------------------------------------------
/// Change the file specified with a new path.
@@ -636,22 +636,11 @@ public:
/// exist, the contents of \a src_path will be copied to \a dst_path
/// unchanged.
///
- /// @param[in] src_path
- /// Input path to be resolved.
- ///
- /// @param[in] dst_path
- /// Buffer to store the resolved path.
- ///
- /// @param[in] dst_len
- /// Size of the buffer pointed to by dst_path.
- ///
- /// @result
- /// The number of characters required to write the resolved path. If the
- /// resolved path doesn't fit in dst_len, dst_len-1 characters will
- /// be written to \a dst_path, but the actual required length will still be returned.
+ /// @param[in] path
+ /// Input path to be resolved, in the form of a llvm::SmallString or similar.
//------------------------------------------------------------------
- static size_t
- Resolve (const char *src_path, char *dst_path, size_t dst_len);
+ static void
+ Resolve (llvm::SmallVectorImpl<char> &path);
FileSpec
CopyByAppendingPathComponent (const char *new_path) const;
@@ -679,18 +668,9 @@ public:
///
/// @param[in] dst_path
/// Buffer to store the resolved path.
- ///
- /// @param[in] dst_len
- /// Size of the buffer pointed to by dst_path.
- ///
- /// @result
- /// The number of characters required to write the resolved path, or 0 if
- /// the user name could not be found. If the
- /// resolved path doesn't fit in dst_len, dst_len-1 characters will
- /// be written to \a dst_path, but the actual required length will still be returned.
//------------------------------------------------------------------
- static size_t
- ResolveUsername (const char *src_path, char *dst_path, size_t dst_len);
+ static void
+ ResolveUsername (llvm::SmallVectorImpl<char> &path);
static size_t
ResolvePartialUsername (const char *partial_name, StringList &matches);
Modified: lldb/trunk/source/API/SBFileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFileSpec.cpp?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/source/API/SBFileSpec.cpp (original)
+++ lldb/trunk/source/API/SBFileSpec.cpp Thu Aug 7 12:33:36 2014
@@ -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 *
Modified: lldb/trunk/source/Commands/CommandCompletions.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandCompletions.cpp?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/source/Commands/CommandCompletions.cpp (original)
+++ lldb/trunk/source/Commands/CommandCompletions.cpp Thu Aug 7 12:33:36 2014
@@ -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, ¶meters);
+ FileSpec::EnumerateDirectory(containing_part.c_str(), true, true, true, DiskFilesOrDirectoriesCallback, ¶meters);
return matches.GetSize();
}
Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Thu Aug 7 12:33:36 2014
@@ -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)
- {
- // 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;
+ llvm::StringRef path_str(path.data());
+ size_t slash_pos = path_str.find_first_of("/", 1);
+ if (slash_pos == 1)
+ {
+ // 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')
- {
- home_dir = GetCachedGlobTildeSlash();
+ 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)
+ {
+ // 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
#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,
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(&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]);
+ llvm::StringRef resolve_path_ref(normalized.c_str());
+ 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(normalized.c_str());
}
//----------------------------------------------------------------------
@@ -732,11 +673,7 @@ FileSpec::GetPath (bool denormalize) con
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());
}
Modified: lldb/trunk/source/Host/common/Host.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/Host.cpp (original)
+++ lldb/trunk/source/Host/common/Host.cpp Thu Aug 7 12:33:36 2014
@@ -1119,7 +1119,6 @@ Host::GetLLDBPath (PathType path_type, F
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, F
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, F
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, F
#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)
Modified: lldb/trunk/source/Target/TargetList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/TargetList.cpp?rev=215124&r1=215123&r2=215124&view=diff
==============================================================================
--- lldb/trunk/source/Target/TargetList.cpp (original)
+++ lldb/trunk/source/Target/TargetList.cpp Thu Aug 7 12:33:36 2014
@@ -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 &debu
{
// 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;
More information about the lldb-commits
mailing list