[Lldb-commits] [lldb] r292100 - FileSpec: Fix PrependPathComponent("/")

2017-01-16 Thread Pavel Labath via lldb-commits
Author: labath
Date: Mon Jan 16 04:07:02 2017
New Revision: 292100

URL: http://llvm.org/viewvc/llvm-project?rev=292100&view=rev
Log:
FileSpec: Fix PrependPathComponent("/")

Summary:
PrependPathComponent was unconditionally inserting path separators between the
path components. This is not correct if the prepended path is "/", which caused
problems down the line. Fix the function to use the same algorithm as
AppendPathComponent and add a test. This fixes one part of llvm.org/pr31611.

Reviewers: clayborg, zturner

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Host/common/FileSpec.cpp
lldb/trunk/unittests/Host/FileSpecTest.cpp

Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=292100&r1=292099&r2=292100&view=diff
==
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Mon Jan 16 04:07:02 2017
@@ -1248,6 +1248,22 @@ ConstString FileSpec::GetLastPathCompone
   return ConstString();
 }
 
+static std::string
+join_path_components(FileSpec::PathSyntax syntax,
+ const std::vector components) {
+  std::string result;
+  for (size_t i = 0; i < components.size(); ++i) {
+if (components[i].empty())
+  continue;
+result += components[i];
+if (i != components.size() - 1 &&
+!IsPathSeparator(components[i].back(), syntax))
+  result += GetPreferredPathSeparator(syntax);
+  }
+
+  return result;
+}
+
 void FileSpec::PrependPathComponent(llvm::StringRef component) {
   if (component.empty())
 return;
@@ -1258,16 +1274,9 @@ void FileSpec::PrependPathComponent(llvm
 return;
   }
 
-  char sep = GetPreferredPathSeparator(m_syntax);
-  std::string result;
-  if (m_filename.IsEmpty())
-result = llvm::join_items(sep, component, m_directory.GetStringRef());
-  else if (m_directory.IsEmpty())
-result = llvm::join_items(sep, component, m_filename.GetStringRef());
-  else
-result = llvm::join_items(sep, component, m_directory.GetStringRef(),
-  m_filename.GetStringRef());
-
+  std::string result =
+  join_path_components(m_syntax, {component, m_directory.GetStringRef(),
+  m_filename.GetStringRef()});
   SetFile(result, resolve);
 }
 
@@ -1279,23 +1288,12 @@ void FileSpec::AppendPathComponent(llvm:
   if (component.empty())
 return;
 
-  std::string result;
-  if (!m_directory.IsEmpty()) {
-result += m_directory.GetStringRef();
-if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-  result += GetPreferredPathSeparator(m_syntax);
-  }
-
-  if (!m_filename.IsEmpty()) {
-result += m_filename.GetStringRef();
-if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax))
-  result += GetPreferredPathSeparator(m_syntax);
-  }
-
   component = component.drop_while(
   [this](char c) { return IsPathSeparator(c, m_syntax); });
 
-  result += component;
+  std::string result =
+  join_path_components(m_syntax, {m_directory.GetStringRef(),
+  m_filename.GetStringRef(), component});
 
   SetFile(result, false, m_syntax);
 }

Modified: lldb/trunk/unittests/Host/FileSpecTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/FileSpecTest.cpp?rev=292100&r1=292099&r2=292100&view=diff
==
--- lldb/trunk/unittests/Host/FileSpecTest.cpp (original)
+++ lldb/trunk/unittests/Host/FileSpecTest.cpp Mon Jan 16 04:07:02 2017
@@ -109,6 +109,28 @@ TEST(FileSpecTest, CopyByAppendingPathCo
   EXPECT_STREQ("bar", fs.GetFilename().GetCString());
 }
 
+TEST(FileSpecTest, PrependPathComponent) {
+  FileSpec fs_posix("foo", false, FileSpec::ePathSyntaxPosix);
+  fs_posix.PrependPathComponent("/bar");
+  EXPECT_STREQ("/bar/foo", fs_posix.GetCString());
+
+  FileSpec fs_posix_2("foo/bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_2.PrependPathComponent("/baz");
+  EXPECT_STREQ("/baz/foo/bar", fs_posix_2.GetCString());
+
+  FileSpec fs_windows("baz", false, FileSpec::ePathSyntaxWindows);
+  fs_windows.PrependPathComponent("F:\\bar");
+  EXPECT_STREQ("F:\\bar\\baz", fs_windows.GetCString());
+
+  FileSpec fs_posix_root("bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_root.PrependPathComponent("/");
+  EXPECT_STREQ("/bar", fs_posix_root.GetCString());
+
+  FileSpec fs_windows_root("bar", false, FileSpec::ePathSyntaxWindows);
+  fs_windows_root.PrependPathComponent("F:\\");
+  EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
+}
+
 static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
 bool remove_backup_dots, bool result) {
   EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
@@ 

[Lldb-commits] [PATCH] D28677: FileSpec: Fix PrependPathComponent("/")

2017-01-16 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL292100: FileSpec: Fix PrependPathComponent("/") (authored by 
labath).

Changed prior to commit:
  https://reviews.llvm.org/D28677?vs=84294&id=84529#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28677

Files:
  lldb/trunk/source/Host/common/FileSpec.cpp
  lldb/trunk/unittests/Host/FileSpecTest.cpp

Index: lldb/trunk/unittests/Host/FileSpecTest.cpp
===
--- lldb/trunk/unittests/Host/FileSpecTest.cpp
+++ lldb/trunk/unittests/Host/FileSpecTest.cpp
@@ -109,6 +109,28 @@
   EXPECT_STREQ("bar", fs.GetFilename().GetCString());
 }
 
+TEST(FileSpecTest, PrependPathComponent) {
+  FileSpec fs_posix("foo", false, FileSpec::ePathSyntaxPosix);
+  fs_posix.PrependPathComponent("/bar");
+  EXPECT_STREQ("/bar/foo", fs_posix.GetCString());
+
+  FileSpec fs_posix_2("foo/bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_2.PrependPathComponent("/baz");
+  EXPECT_STREQ("/baz/foo/bar", fs_posix_2.GetCString());
+
+  FileSpec fs_windows("baz", false, FileSpec::ePathSyntaxWindows);
+  fs_windows.PrependPathComponent("F:\\bar");
+  EXPECT_STREQ("F:\\bar\\baz", fs_windows.GetCString());
+
+  FileSpec fs_posix_root("bar", false, FileSpec::ePathSyntaxPosix);
+  fs_posix_root.PrependPathComponent("/");
+  EXPECT_STREQ("/bar", fs_posix_root.GetCString());
+
+  FileSpec fs_windows_root("bar", false, FileSpec::ePathSyntaxWindows);
+  fs_windows_root.PrependPathComponent("F:\\");
+  EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
+}
+
 static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
 bool remove_backup_dots, bool result) {
   EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
@@ -283,4 +305,4 @@
   EXPECT_EQ("foo", llvm::formatv("{0}", F).str());
   EXPECT_EQ("foo", llvm::formatv("{0:F}", F).str());
   EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
-}
\ No newline at end of file
+}
Index: lldb/trunk/source/Host/common/FileSpec.cpp
===
--- lldb/trunk/source/Host/common/FileSpec.cpp
+++ lldb/trunk/source/Host/common/FileSpec.cpp
@@ -1248,6 +1248,22 @@
   return ConstString();
 }
 
+static std::string
+join_path_components(FileSpec::PathSyntax syntax,
+ const std::vector components) {
+  std::string result;
+  for (size_t i = 0; i < components.size(); ++i) {
+if (components[i].empty())
+  continue;
+result += components[i];
+if (i != components.size() - 1 &&
+!IsPathSeparator(components[i].back(), syntax))
+  result += GetPreferredPathSeparator(syntax);
+  }
+
+  return result;
+}
+
 void FileSpec::PrependPathComponent(llvm::StringRef component) {
   if (component.empty())
 return;
@@ -1258,16 +1274,9 @@
 return;
   }
 
-  char sep = GetPreferredPathSeparator(m_syntax);
-  std::string result;
-  if (m_filename.IsEmpty())
-result = llvm::join_items(sep, component, m_directory.GetStringRef());
-  else if (m_directory.IsEmpty())
-result = llvm::join_items(sep, component, m_filename.GetStringRef());
-  else
-result = llvm::join_items(sep, component, m_directory.GetStringRef(),
-  m_filename.GetStringRef());
-
+  std::string result =
+  join_path_components(m_syntax, {component, m_directory.GetStringRef(),
+  m_filename.GetStringRef()});
   SetFile(result, resolve);
 }
 
@@ -1279,23 +1288,12 @@
   if (component.empty())
 return;
 
-  std::string result;
-  if (!m_directory.IsEmpty()) {
-result += m_directory.GetStringRef();
-if (!IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
-  result += GetPreferredPathSeparator(m_syntax);
-  }
-
-  if (!m_filename.IsEmpty()) {
-result += m_filename.GetStringRef();
-if (!IsPathSeparator(m_filename.GetStringRef().back(), m_syntax))
-  result += GetPreferredPathSeparator(m_syntax);
-  }
-
   component = component.drop_while(
   [this](char c) { return IsPathSeparator(c, m_syntax); });
 
-  result += component;
+  std::string result =
+  join_path_components(m_syntax, {m_directory.GetStringRef(),
+  m_filename.GetStringRef(), component});
 
   SetFile(result, false, m_syntax);
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] r292106 - Fix windows unit tests

2017-01-16 Thread Pavel Labath via lldb-commits
Author: labath
Date: Mon Jan 16 06:15:42 2017
New Revision: 292106

URL: http://llvm.org/viewvc/llvm-project?rev=292106&view=rev
Log:
Fix windows unit tests

The unit test I added in the previous commit discovered a bug in
PrependPathComponent on windows -- it was calling SetFile with the host native
path syntax, whereas it should be explicitly specifying the path syntax (as
AppendPathComponent does). This fixes it.

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

Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=292106&r1=292105&r2=292106&view=diff
==
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Mon Jan 16 06:15:42 2017
@@ -1277,7 +1277,7 @@ void FileSpec::PrependPathComponent(llvm
   std::string result =
   join_path_components(m_syntax, {component, m_directory.GetStringRef(),
   m_filename.GetStringRef()});
-  SetFile(result, resolve);
+  SetFile(result, resolve, m_syntax);
 }
 
 void FileSpec::PrependPathComponent(const FileSpec &new_path) {


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D28775: [cmake] Make lldb build with the android ndk toolchain file

2017-01-16 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: tberghammer, danalbert.
labath added a subscriber: lldb-commits.
Herald added subscribers: mgorny, srhines.

The NDK cmake toolchain file defines CMAKE_SYSTEM_NAME=Android, so switch the
build to use that. I have also updated the in-tree toolchain file to do that
(instead of defining __ANDROID_NDK__), so it can still be used to build.
After migrating the last bits of non-toolchainy bits out of the in-tree
toolchain, I intend to delete it.


https://reviews.llvm.org/D28775

Files:
  CMakeLists.txt
  cmake/LLDBDependencies.cmake
  cmake/modules/LLDBConfig.cmake
  cmake/platforms/Android.cmake
  source/Host/CMakeLists.txt
  source/Plugins/Process/CMakeLists.txt
  tools/lldb-server/CMakeLists.txt

Index: tools/lldb-server/CMakeLists.txt
===
--- tools/lldb-server/CMakeLists.txt
+++ tools/lldb-server/CMakeLists.txt
@@ -77,7 +77,7 @@
   )
 
 # Linux-only libraries
-if ( CMAKE_SYSTEM_NAME MATCHES "Linux" )
+if ( CMAKE_SYSTEM_NAME MATCHES "Linux|Android" )
   list(APPEND LLDB_USED_LIBS
 lldbPluginProcessLinux
 lldbPluginProcessPOSIX
Index: source/Plugins/Process/CMakeLists.txt
===
--- source/Plugins/Process/CMakeLists.txt
+++ source/Plugins/Process/CMakeLists.txt
@@ -1,4 +1,4 @@
-if (CMAKE_SYSTEM_NAME MATCHES "Linux")
+if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
Index: source/Host/CMakeLists.txt
===
--- source/Host/CMakeLists.txt
+++ source/Host/CMakeLists.txt
@@ -88,7 +88,7 @@
 posix/PipePosix.cpp
 )
 
-  if (NOT __ANDROID_NDK__)
+  if (NOT (CMAKE_SYSTEM_NAME MATCHES "Android"))
 add_host_subdirectory(posix
   posix/ProcessLauncherPosix.cpp
   )
@@ -110,13 +110,7 @@
   macosx/cfcpp/CFCString.cpp
   )
 
-  elseif (CMAKE_SYSTEM_NAME MATCHES "Linux")
-if (__ANDROID_NDK__)
-  add_host_subdirectory(android
-android/HostInfoAndroid.cpp
-android/LibcGlue.cpp
-)
-endif()
+  elseif (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
 add_host_subdirectory(linux
   linux/AbstractSocket.cpp
   linux/Host.cpp
@@ -126,7 +120,12 @@
   linux/ProcessLauncherLinux.cpp
   linux/ThisThread.cpp
   )
-
+if (CMAKE_SYSTEM_NAME MATCHES "Android")
+  add_host_subdirectory(android
+android/HostInfoAndroid.cpp
+android/LibcGlue.cpp
+)
+endif()
   elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
 add_host_subdirectory(freebsd
   freebsd/Host.cpp
Index: cmake/platforms/Android.cmake
===
--- cmake/platforms/Android.cmake
+++ cmake/platforms/Android.cmake
@@ -29,13 +29,12 @@
  return()
 endif()
 
-set( CMAKE_SYSTEM_NAME Linux )
+set( CMAKE_SYSTEM_NAME Android )
 include( CMakeForceCompiler )
 
 # flags and definitions
 add_definitions( -DANDROID -DLLDB_DISABLE_LIBEDIT )
 set( ANDROID True )
-set( __ANDROID_NDK__ True )
 set( LLDB_DEFAULT_DISABLE_LIBEDIT True )
 
 # linking lldb-server statically for Android avoids the need to ship two
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -13,14 +13,12 @@
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   set(LLDB_DEFAULT_DISABLE_PYTHON 0)
   set(LLDB_DEFAULT_DISABLE_CURSES 1)
+elseif (CMAKE_SYSTEM_NAME MATCHES "Android" )
+  set(LLDB_DEFAULT_DISABLE_PYTHON 1)
+  set(LLDB_DEFAULT_DISABLE_CURSES 1)
 else()
-  if ( __ANDROID_NDK__ )
-set(LLDB_DEFAULT_DISABLE_PYTHON 1)
-set(LLDB_DEFAULT_DISABLE_CURSES 1)
-  else()
-set(LLDB_DEFAULT_DISABLE_PYTHON 0)
-set(LLDB_DEFAULT_DISABLE_CURSES 0)
-  endif()
+  set(LLDB_DEFAULT_DISABLE_PYTHON 0)
+  set(LLDB_DEFAULT_DISABLE_CURSES 0)
 endif()
 
 set(LLDB_DISABLE_PYTHON ${LLDB_DEFAULT_DISABLE_PYTHON} CACHE BOOL
@@ -354,10 +352,7 @@
 
 # Figure out if lldb could use lldb-server.  If so, then we'll
 # ensure we build lldb-server when an lldb target is being built.
-if ((CMAKE_SYSTEM_NAME MATCHES "Darwin") OR
-(CMAKE_SYSTEM_NAME MATCHES "FreeBSD") OR
-(CMAKE_SYSTEM_NAME MATCHES "Linux") OR
-(CMAKE_SYSTEM_NAME MATCHES "NetBSD"))
+if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
 set(LLDB_CAN_USE_LLDB_SERVER 1)
 else()
 set(LLDB_CAN_USE_LLDB_SERVER 0)
Index: cmake/LLDBDependencies.cmake
===
--- cmake/LLDBDependencies.cmake
+++ cmake/LLDBDependencies.cmake
@@ -97,7 +97,7 @@
 endif ()
 
 # Linux-only libraries
-if ( CMAKE_SYSTEM_NAME MATCHES "Linux" )
+if ( CMAKE_SYSTEM_NAME MATCHES "Linux|Android" )
   list(APPEND LLDB_USED_LIBS
 lldbPluginProcessLinux
 lldbPluginProcessPOSIX
Index: CMakeLists.