[Lldb-commits] [lldb] r282537 - Update FileSpec's interface to use StringRefs.

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 27 13:48:37 PDT 2016


Author: zturner
Date: Tue Sep 27 15:48:37 2016
New Revision: 282537

URL: http://llvm.org/viewvc/llvm-project?rev=282537&view=rev
Log:
Update FileSpec's interface to use StringRefs.

Differential Revision: https://reviews.llvm.org/D24936

Modified:
    lldb/trunk/include/lldb/Host/FileSpec.h
    lldb/trunk/source/Host/common/FileSpec.cpp

Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=282537&r1=282536&r2=282537&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Tue Sep 27 15:48:37 2016
@@ -80,16 +80,10 @@ public:
   ///
   /// @see FileSpec::SetFile (const char *path, bool resolve)
   //------------------------------------------------------------------
-  // TODO: Convert this constructor to use a StringRef.
-  explicit FileSpec(const char *path, bool resolve_path,
+  explicit FileSpec(llvm::StringRef path, bool resolve_path,
                     PathSyntax syntax = ePathSyntaxHostNative);
 
-  explicit FileSpec(const char *path, bool resolve_path, ArchSpec arch);
-
-  explicit FileSpec(const std::string &path, bool resolve_path,
-                    PathSyntax syntax = ePathSyntaxHostNative);
-
-  explicit FileSpec(const std::string &path, bool resolve_path, ArchSpec arch);
+  explicit FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   //------------------------------------------------------------------
   /// Copy constructor
@@ -657,15 +651,10 @@ public:
   ///     If \b true, then we will try to resolve links the path using
   ///     the static FileSpec::Resolve.
   //------------------------------------------------------------------
-  void SetFile(const char *path, bool resolve_path,
-               PathSyntax syntax = ePathSyntaxHostNative);
-
-  void SetFile(const char *path, bool resolve_path, ArchSpec arch);
-
-  void SetFile(const std::string &path, bool resolve_path,
+  void SetFile(llvm::StringRef path, bool resolve_path,
                PathSyntax syntax = ePathSyntaxHostNative);
 
-  void SetFile(const std::string &path, bool resolve_path, ArchSpec arch);
+  void SetFile(llvm::StringRef path, bool resolve_path, ArchSpec arch);
 
   bool IsResolved() const { return m_is_resolved; }
 
@@ -709,20 +698,13 @@ public:
   //------------------------------------------------------------------
   static void Resolve(llvm::SmallVectorImpl<char> &path);
 
-  FileSpec CopyByAppendingPathComponent(const char *new_path) const;
-
+  FileSpec CopyByAppendingPathComponent(llvm::StringRef component) const;
   FileSpec CopyByRemovingLastPathComponent() const;
 
-  void PrependPathComponent(const char *new_path);
-
-  void PrependPathComponent(const std::string &new_path);
-
+  void PrependPathComponent(llvm::StringRef component);
   void PrependPathComponent(const FileSpec &new_path);
 
-  void AppendPathComponent(const char *new_path);
-
-  void AppendPathComponent(const std::string &new_path);
-
+  void AppendPathComponent(llvm::StringRef component);
   void AppendPathComponent(const FileSpec &new_path);
 
   void RemoveLastPathComponent();
@@ -746,7 +728,7 @@ public:
   //------------------------------------------------------------------
   static void ResolveUsername(llvm::SmallVectorImpl<char> &path);
 
-  static size_t ResolvePartialUsername(const char *partial_name,
+  static size_t ResolvePartialUsername(llvm::StringRef partial_name,
                                        StringList &matches);
 
   enum EnumerateDirectoryResult {
@@ -763,7 +745,7 @@ public:
       void *baton, FileType file_type, const FileSpec &spec);
 
   static EnumerateDirectoryResult
-  EnumerateDirectory(const char *dir_path, bool find_directories,
+  EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
                      bool find_files, bool find_other,
                      EnumerateDirectoryCallbackType callback,
                      void *callback_baton);
@@ -773,7 +755,7 @@ public:
       DirectoryCallback;
 
   static EnumerateDirectoryResult
-  ForEachItemInDirectory(const char *dir_path,
+  ForEachItemInDirectory(llvm::StringRef dir_path,
                          DirectoryCallback const &callback);
 
 protected:

Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=282537&r1=282536&r2=282537&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Tue Sep 27 15:48:37 2016
@@ -37,6 +37,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Utility/CleanUp.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/FileSystem.h"
@@ -58,7 +59,7 @@ const char *GetPathSeparators(FileSpec::
   return PathSyntaxIsPosix(syntax) ? "/" : "\\/";
 }
 
-char GetPrefferedPathSeparator(FileSpec::PathSyntax syntax) {
+char GetPreferredPathSeparator(FileSpec::PathSyntax syntax) {
   return GetPathSeparators(syntax)[0];
 }
 
@@ -228,27 +229,27 @@ void FileSpec::ResolveUsername(llvm::Sma
 #endif
 }
 
-size_t FileSpec::ResolvePartialUsername(const char *partial_name,
+size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name,
                                         StringList &matches) {
 #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER
   size_t extant_entries = matches.GetSize();
 
   setpwent();
   struct passwd *user_entry;
-  const char *name_start = partial_name + 1;
+  partial_name = partial_name.drop_front();
   std::set<std::string> name_list;
 
   while ((user_entry = getpwent()) != NULL) {
-    if (strstr(user_entry->pw_name, name_start) == user_entry->pw_name) {
+    if (llvm::StringRef(user_entry->pw_name).startswith(partial_name)) {
       std::string tmp_buf("~");
       tmp_buf.append(user_entry->pw_name);
       tmp_buf.push_back('/');
       name_list.insert(tmp_buf);
     }
   }
-  std::set<std::string>::iterator pos, end = name_list.end();
-  for (pos = name_list.begin(); pos != end; pos++) {
-    matches.AppendString((*pos).c_str());
+
+  for (auto &name : name_list) {
+    matches.AppendString(name);
   }
   return matches.GetSize() - extant_entries;
 #else
@@ -284,23 +285,15 @@ FileSpec::FileSpec()
 // Default constructor that can take an optional full path to a
 // file on disk.
 //------------------------------------------------------------------
-FileSpec::FileSpec(const char *pathname, bool resolve_path, PathSyntax syntax)
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, PathSyntax syntax)
     : m_directory(), m_filename(), m_is_resolved(false), m_syntax(syntax) {
-  if (pathname && pathname[0])
-    SetFile(pathname, resolve_path, syntax);
+  SetFile(path, resolve_path, syntax);
 }
 
-FileSpec::FileSpec(const char *pathname, bool resolve_path, ArchSpec arch)
-    : FileSpec{pathname, resolve_path, arch.GetTriple().isOSWindows()
-                                           ? ePathSyntaxWindows
-                                           : ePathSyntaxPosix} {}
-
-FileSpec::FileSpec(const std::string &path, bool resolve_path,
-                   PathSyntax syntax)
-    : FileSpec{path.c_str(), resolve_path, syntax} {}
-
-FileSpec::FileSpec(const std::string &path, bool resolve_path, ArchSpec arch)
-    : FileSpec{path.c_str(), resolve_path, arch} {}
+FileSpec::FileSpec(llvm::StringRef path, bool resolve_path, ArchSpec arch)
+    : FileSpec{path, resolve_path, arch.GetTriple().isOSWindows()
+                                       ? ePathSyntaxWindows
+                                       : ePathSyntaxPosix} {}
 
 //------------------------------------------------------------------
 // Copy constructor
@@ -340,7 +333,12 @@ const FileSpec &FileSpec::operator=(cons
 // be split up into a directory and filename and stored as uniqued
 // string values for quick comparison and efficient memory usage.
 //------------------------------------------------------------------
-void FileSpec::SetFile(const char *pathname, bool resolve, PathSyntax syntax) {
+void FileSpec::SetFile(llvm::StringRef pathname, bool resolve,
+                       PathSyntax syntax) {
+  // CLEANUP: Use StringRef for string handling.  This function is kind of a
+  // mess and the unclear semantics of RootDirStart and ParentPathEnd make
+  // it very difficult to understand this function.  There's no reason this
+  // function should be particularly complicated or difficult to understand.
   m_filename.Clear();
   m_directory.Clear();
   m_is_resolved = false;
@@ -348,7 +346,7 @@ void FileSpec::SetFile(const char *pathn
                  ? FileSystem::GetNativePathSyntax()
                  : syntax;
 
-  if (pathname == NULL || pathname[0] == '\0')
+  if (pathname.empty())
     return;
 
   llvm::SmallString<64> resolved(pathname);
@@ -382,20 +380,10 @@ void FileSpec::SetFile(const char *pathn
                            : resolve_path_ref.substr(filename_begin));
 }
 
-void FileSpec::SetFile(const char *pathname, bool resolve, ArchSpec arch) {
-  return SetFile(pathname, resolve, arch.GetTriple().isOSWindows()
-                                        ? ePathSyntaxWindows
-                                        : ePathSyntaxPosix);
-}
-
-void FileSpec::SetFile(const std::string &pathname, bool resolve,
-                       PathSyntax syntax) {
-  return SetFile(pathname.c_str(), resolve, syntax);
-}
-
-void FileSpec::SetFile(const std::string &pathname, bool resolve,
-                       ArchSpec arch) {
-  return SetFile(pathname.c_str(), resolve, arch);
+void FileSpec::SetFile(llvm::StringRef path, bool resolve, ArchSpec arch) {
+  return SetFile(path, resolve, arch.GetTriple().isOSWindows()
+                                    ? ePathSyntaxWindows
+                                    : ePathSyntaxPosix);
 }
 
 //----------------------------------------------------------------------
@@ -670,7 +658,7 @@ void FileSpec::Dump(Stream *s) const {
   if (s) {
     std::string path{GetPath(true)};
     s->PutCString(path.c_str());
-    char path_separator = GetPrefferedPathSeparator(m_syntax);
+    char path_separator = GetPreferredPathSeparator(m_syntax);
     if (!m_filename && !path.empty() && path.back() != path_separator)
       s->PutChar(path_separator);
   }
@@ -692,6 +680,7 @@ bool FileSpec::Readable() const {
 }
 
 bool FileSpec::ResolveExecutableLocation() {
+  // CLEANUP: Use StringRef for string handling.
   if (!m_directory) {
     const char *file_cstr = m_filename.GetCString();
     if (file_cstr) {
@@ -862,7 +851,7 @@ void FileSpec::GetPath(llvm::SmallVector
               m_directory.GetStringRef().end());
   if (m_directory && m_filename &&
       !IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-    path.insert(path.end(), GetPrefferedPathSeparator(m_syntax));
+    path.insert(path.end(), GetPreferredPathSeparator(m_syntax));
   path.append(m_filename.GetStringRef().begin(),
               m_filename.GetStringRef().end());
   Normalize(path, m_syntax);
@@ -1023,9 +1012,11 @@ size_t FileSpec::ReadFileLines(STLString
 }
 
 FileSpec::EnumerateDirectoryResult
-FileSpec::ForEachItemInDirectory(const char *dir_path,
+FileSpec::ForEachItemInDirectory(llvm::StringRef dir_path,
                                  DirectoryCallback const &callback) {
-  if (dir_path && dir_path[0]) {
+  if (dir_path.empty())
+    return eEnumerateDirectoryResultNext;
+
 #ifdef _WIN32
     std::string szDir(dir_path);
     szDir += "\\*";
@@ -1065,55 +1056,51 @@ FileSpec::ForEachItemInDirectory(const c
         continue;
       }
 
-      std::vector<char> child_path(PATH_MAX);
-      const int child_path_len =
-          ::snprintf(child_path.data(), child_path.size(), "%s\\%s", dir_path,
-                     fileName.c_str());
-      if (child_path_len < (int)(child_path.size() - 1)) {
-        // Don't resolve the file type or path
-        FileSpec child_path_spec(child_path.data(), false);
-
-        EnumerateDirectoryResult result = callback(file_type, child_path_spec);
-
-        switch (result) {
-        case eEnumerateDirectoryResultNext:
-          // Enumerate next entry in the current directory. We just
-          // exit this switch and will continue enumerating the
-          // current directory as we currently are...
-          break;
-
-        case eEnumerateDirectoryResultEnter: // Recurse into the current entry
-                                             // if it is a directory or symlink,
-                                             // or next if not
-          if (FileSpec::ForEachItemInDirectory(child_path.data(), callback) ==
-              eEnumerateDirectoryResultQuit) {
-            // The subdirectory returned Quit, which means to
-            // stop all directory enumerations at all levels.
-            return eEnumerateDirectoryResultQuit;
-          }
-          break;
-
-        case eEnumerateDirectoryResultExit: // Exit from the current directory
-                                            // at the current level.
-          // Exit from this directory level and tell parent to
-          // keep enumerating.
-          return eEnumerateDirectoryResultNext;
+      std::string child_path = llvm::join_items("\\", dir_path, fileName);
+      // Don't resolve the file type or path
+      FileSpec child_path_spec(child_path.data(), false);
+
+      EnumerateDirectoryResult result = callback(file_type, child_path_spec);
+
+      switch (result) {
+      case eEnumerateDirectoryResultNext:
+        // Enumerate next entry in the current directory. We just
+        // exit this switch and will continue enumerating the
+        // current directory as we currently are...
+        break;
 
-        case eEnumerateDirectoryResultQuit: // Stop directory enumerations at
-                                            // any level
+      case eEnumerateDirectoryResultEnter: // Recurse into the current entry
+                                           // if it is a directory or symlink,
+                                           // or next if not
+        if (FileSpec::ForEachItemInDirectory(child_path.data(), callback) ==
+            eEnumerateDirectoryResultQuit) {
+          // The subdirectory returned Quit, which means to
+          // stop all directory enumerations at all levels.
           return eEnumerateDirectoryResultQuit;
         }
+        break;
+
+      case eEnumerateDirectoryResultExit: // Exit from the current directory
+                                          // at the current level.
+        // Exit from this directory level and tell parent to
+        // keep enumerating.
+        return eEnumerateDirectoryResultNext;
+
+      case eEnumerateDirectoryResultQuit: // Stop directory enumerations at
+                                          // any level
+        return eEnumerateDirectoryResultQuit;
       }
     } while (FindNextFileW(hFind, &ffd) != 0);
 
     FindClose(hFind);
 #else
-    lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_path), NULL,
-                                                   closedir);
-    if (dir_path_dir.is_valid()) {
-      char dir_path_last_char = dir_path[strlen(dir_path) - 1];
+  std::string dir_string(dir_path);
+  lldb_utility::CleanUp<DIR *, int> dir_path_dir(opendir(dir_string.c_str()),
+                                                 NULL, closedir);
+  if (dir_path_dir.is_valid()) {
+    char dir_path_last_char = dir_path.back();
 
-      long path_max = fpathconf(dirfd(dir_path_dir.get()), _PC_NAME_MAX);
+    long path_max = fpathconf(dirfd(dir_path_dir.get()), _PC_NAME_MAX);
 #if defined(__APPLE_) && defined(__DARWIN_MAXPATHLEN)
       if (path_max < __DARWIN_MAXPATHLEN)
         path_max = __DARWIN_MAXPATHLEN;
@@ -1169,18 +1156,13 @@ FileSpec::ForEachItemInDirectory(const c
 #endif
         }
 
-        char child_path[PATH_MAX];
-
+        std::string child_path;
         // Don't make paths with "/foo//bar", that just confuses everybody.
-        int child_path_len;
         if (dir_path_last_char == '/')
-          child_path_len = ::snprintf(child_path, sizeof(child_path), "%s%s",
-                                      dir_path, dp->d_name);
+          child_path = llvm::join_items("", dir_path, dp->d_name);
         else
-          child_path_len = ::snprintf(child_path, sizeof(child_path), "%s/%s",
-                                      dir_path, dp->d_name);
+          child_path = llvm::join_items('/', dir_path, dp->d_name);
 
-        if (child_path_len < (int)(sizeof(child_path) - 1)) {
           // Don't resolve the file type or path
           FileSpec child_path_spec(child_path, false);
 
@@ -1221,21 +1203,19 @@ FileSpec::ForEachItemInDirectory(const c
               free(buf);
             return eEnumerateDirectoryResultQuit;
           }
-        }
       }
       if (buf) {
         free(buf);
       }
     }
 #endif
-  }
   // By default when exiting a directory, we tell the parent enumeration
   // to continue enumerating.
   return eEnumerateDirectoryResultNext;
 }
 
 FileSpec::EnumerateDirectoryResult
-FileSpec::EnumerateDirectory(const char *dir_path, bool find_directories,
+FileSpec::EnumerateDirectory(llvm::StringRef dir_path, bool find_directories,
                              bool find_files, bool find_other,
                              EnumerateDirectoryCallbackType callback,
                              void *callback_baton) {
@@ -1261,13 +1241,15 @@ FileSpec::EnumerateDirectory(const char
       });
 }
 
-FileSpec FileSpec::CopyByAppendingPathComponent(const char *new_path) const {
+FileSpec
+FileSpec::CopyByAppendingPathComponent(llvm::StringRef component) const {
   FileSpec ret = *this;
-  ret.AppendPathComponent(new_path);
+  ret.AppendPathComponent(component);
   return ret;
 }
 
 FileSpec FileSpec::CopyByRemovingLastPathComponent() const {
+  // CLEANUP: Use StringRef for string handling.
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty())
     return FileSpec("", resolve);
@@ -1291,6 +1273,7 @@ FileSpec FileSpec::CopyByRemovingLastPat
 }
 
 ConstString FileSpec::GetLastPathComponent() const {
+  // CLEANUP: Use StringRef for string handling.
   if (m_filename)
     return m_filename;
   if (m_directory) {
@@ -1321,61 +1304,56 @@ ConstString FileSpec::GetLastPathCompone
   return ConstString();
 }
 
-void FileSpec::PrependPathComponent(const char *new_path) {
-  if (!new_path)
+void FileSpec::PrependPathComponent(llvm::StringRef component) {
+  if (component.empty())
     return;
+
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty()) {
-    SetFile(new_path, resolve);
+    SetFile(component, resolve);
     return;
   }
-  StreamString stream;
+
+  char sep = GetPreferredPathSeparator(m_syntax);
+  std::string result;
   if (m_filename.IsEmpty())
-    stream.Printf("%s/%s", new_path, m_directory.GetCString());
+    result = llvm::join_items(sep, component, m_directory.GetStringRef());
   else if (m_directory.IsEmpty())
-    stream.Printf("%s/%s", new_path, m_filename.GetCString());
+    result = llvm::join_items(sep, component, m_filename.GetStringRef());
   else
-    stream.Printf("%s/%s/%s", new_path, m_directory.GetCString(),
-                  m_filename.GetCString());
-  SetFile(stream.GetData(), resolve);
-}
+    result = llvm::join_items(sep, component, m_directory.GetStringRef(),
+                              m_filename.GetStringRef());
 
-void FileSpec::PrependPathComponent(const std::string &new_path) {
-  return PrependPathComponent(new_path.c_str());
+  SetFile(result, resolve);
 }
 
 void FileSpec::PrependPathComponent(const FileSpec &new_path) {
   return PrependPathComponent(new_path.GetPath(false));
 }
 
-void FileSpec::AppendPathComponent(const char *new_path) {
-  if (!new_path)
+void FileSpec::AppendPathComponent(llvm::StringRef component) {
+  if (component.empty())
     return;
 
-  StreamString stream;
+  std::string result;
   if (!m_directory.IsEmpty()) {
-    stream.PutCString(m_directory.GetCString());
+    result += m_directory.GetStringRef();
     if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-      stream.PutChar(GetPrefferedPathSeparator(m_syntax));
+      result += GetPreferredPathSeparator(m_syntax);
   }
 
   if (!m_filename.IsEmpty()) {
-    stream.PutCString(m_filename.GetCString());
+    result += m_filename.GetStringRef();
     if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax))
-      stream.PutChar(GetPrefferedPathSeparator(m_syntax));
+      result += GetPreferredPathSeparator(m_syntax);
   }
 
-  while (IsPathSeparator(new_path[0], m_syntax))
-    new_path++;
+  component = component.drop_while(
+      [this](char c) { return IsPathSeparator(c, m_syntax); });
 
-  stream.PutCString(new_path);
+  result += component;
 
-  const bool resolve = false;
-  SetFile(stream.GetData(), resolve, m_syntax);
-}
-
-void FileSpec::AppendPathComponent(const std::string &new_path) {
-  return AppendPathComponent(new_path.c_str());
+  SetFile(result, false, m_syntax);
 }
 
 void FileSpec::AppendPathComponent(const FileSpec &new_path) {
@@ -1383,6 +1361,8 @@ void FileSpec::AppendPathComponent(const
 }
 
 void FileSpec::RemoveLastPathComponent() {
+  // CLEANUP: Use StringRef for string handling.
+
   const bool resolve = false;
   if (m_filename.IsEmpty() && m_directory.IsEmpty()) {
     SetFile("", resolve);




More information about the lldb-commits mailing list