Fix a couple of cornercases in FileSpec + tests

Summary:
This fixes a couple of corner cases in FileSpec, related to AppendPathComponent and
handling of root directory (/) file spec. I add a bunch of unit tests for the new behavior.

Summary of changes:
FileSpec("/bar").GetCString(): before "//bar", after "/bar".
FileSpec("/").CopyByAppendingPathComponent("bar").GetCString(): before "//bar", after "/bar".
FileSpec("C:", ePathSyntaxWindows).CopyByAppendingPathComponent("bar").GetCString(): before "C:/bar", after "C:\bar".

Reviewers: clayborg, zturner

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D18044

llvm-svn: 263207

lldb/source/Host/common/FileSpec.cpp

@@ -918,7 +918,7 @@ void
 FileSpec::GetPath(llvm::SmallVectorImpl<char> &path, bool denormalize) const
 {
     path.append(m_directory.GetStringRef().begin(), m_directory.GetStringRef().end());
-    if (m_directory)
+    if (m_directory && !(m_directory.GetLength() == 1 && m_directory.GetCString()[0] == '/'))
         path.insert(path.end(), '/');
     path.append(m_filename.GetStringRef().begin(), m_filename.GetStringRef().end());
     Normalize(path, m_syntax);
@@ -1331,17 +1331,9 @@ FileSpec::EnumerateDirectory
 FileSpec
 FileSpec::CopyByAppendingPathComponent (const char *new_path)  const
 {
-    const bool resolve = false;
-    if (m_filename.IsEmpty() && m_directory.IsEmpty())
-        return FileSpec(new_path,resolve);
-    StreamString stream;
-    if (m_filename.IsEmpty())
-        stream.Printf("%s/%s",m_directory.GetCString(),new_path);
-    else if (m_directory.IsEmpty())
-        stream.Printf("%s/%s",m_filename.GetCString(),new_path);
-    else
-        stream.Printf("%s/%s/%s",m_directory.GetCString(), m_filename.GetCString(),new_path);
-    return FileSpec(stream.GetData(),resolve);
+    FileSpec ret = *this;
+    ret.AppendPathComponent(new_path);
+    return ret;
 }
 
 FileSpec
@@ -1442,20 +1434,26 @@ void
 FileSpec::AppendPathComponent(const char *new_path)
 {
     if (!new_path) return;
-    const bool resolve = false;
-    if (m_filename.IsEmpty() && m_directory.IsEmpty())
-    {
-        SetFile(new_path, resolve);
-        return;
-    }
+
     StreamString stream;
-    if (m_filename.IsEmpty() || (m_filename.GetLength() == 1 && m_filename.GetCString()[0] == '.'))
-        stream.Printf("%s/%s", m_directory.GetCString(), new_path);
-    else if (m_directory.IsEmpty())
-        stream.Printf("%s/%s", m_filename.GetCString(), new_path);
-    else
-        stream.Printf("%s/%s/%s", m_directory.GetCString(), m_filename.GetCString(), new_path);
-    SetFile(stream.GetData(), resolve);
+    if (!m_directory.IsEmpty())
+    {
+        stream.PutCString(m_directory.GetCString());
+        if (m_directory.GetLength() != 1 || m_directory.GetCString()[0] != '/')
+            stream.PutChar('/');
+    }
+
+    if (!m_filename.IsEmpty())
+    {
+        stream.PutCString(m_filename.GetCString());
+        if (m_filename.GetLength() != 1 || m_filename.GetCString()[0] != '/')
+            stream.PutChar('/');
+    }
+
+    stream.PutCString(new_path);
+
+    const bool resolve = false;
+    SetFile(stream.GetData(), resolve, m_syntax);
 }
 
 void

lldb/unittests/Host/CMakeLists.txt

@@ -1,4 +1,5 @@
 add_lldb_unittest(HostTests
+  FileSpecTest.cpp
   SocketAddressTest.cpp
   SocketTest.cpp
   SymbolsTest.cpp

lldb/unittests/Host/FileSpecTest.cpp

@@ -0,0 +1,94 @@
+//===-- FileSpecTest.cpp ----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Host/FileSpec.h"
+
+using namespace lldb_private;
+
+TEST(FileSpecTest, FileAndDirectoryComponents)
+{
+    FileSpec fs_posix("/foo/bar", false, FileSpec::ePathSyntaxPosix);
+    EXPECT_STREQ("/foo/bar", fs_posix.GetCString());
+    EXPECT_STREQ("/foo", fs_posix.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_posix.GetFilename().GetCString());
+
+    FileSpec fs_windows("F:\\bar", false, FileSpec::ePathSyntaxWindows);
+    EXPECT_STREQ("F:\\bar", fs_windows.GetCString());
+    EXPECT_STREQ("F:", fs_windows.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString());
+
+    FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix);
+    EXPECT_STREQ("/", fs_posix_root.GetCString());
+    EXPECT_EQ(nullptr, fs_posix_root.GetDirectory().GetCString());
+    EXPECT_STREQ("/", fs_posix_root.GetFilename().GetCString());
+
+    FileSpec fs_windows_root("F:", false, FileSpec::ePathSyntaxWindows);
+    EXPECT_STREQ("F:", fs_windows_root.GetCString());
+    EXPECT_EQ(nullptr, fs_windows_root.GetDirectory().GetCString());
+    EXPECT_STREQ("F:", fs_windows_root.GetFilename().GetCString());
+
+    FileSpec fs_posix_long("/foo/bar/baz", false, FileSpec::ePathSyntaxPosix);
+    EXPECT_STREQ("/foo/bar/baz", fs_posix_long.GetCString());
+    EXPECT_STREQ("/foo/bar", fs_posix_long.GetDirectory().GetCString());
+    EXPECT_STREQ("baz", fs_posix_long.GetFilename().GetCString());
+
+    FileSpec fs_windows_long("F:\\bar\\baz", false, FileSpec::ePathSyntaxWindows);
+    EXPECT_STREQ("F:\\bar\\baz", fs_windows_long.GetCString());
+    // We get "F:/bar" instead.
+    // EXPECT_STREQ("F:\\bar", fs_windows_long.GetDirectory().GetCString());
+    EXPECT_STREQ("baz", fs_windows_long.GetFilename().GetCString());
+
+    FileSpec fs_posix_trailing_slash("/foo/bar/", false, FileSpec::ePathSyntaxPosix);
+    EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
+    EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
+    EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
+
+    FileSpec fs_windows_trailing_slash("F:\\bar\\", false, FileSpec::ePathSyntaxWindows);
+    EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
+    // We get "F:/bar" instead.
+    // EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetDirectory().GetCString());
+    EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString());
+}
+
+TEST(FileSpecTest, AppendPathComponent)
+{
+    FileSpec fs_posix("/foo", false, FileSpec::ePathSyntaxPosix);
+    fs_posix.AppendPathComponent("bar");
+    EXPECT_STREQ("/foo/bar", fs_posix.GetCString());
+    EXPECT_STREQ("/foo", fs_posix.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_posix.GetFilename().GetCString());
+
+    FileSpec fs_windows("F:", false, FileSpec::ePathSyntaxWindows);
+    fs_windows.AppendPathComponent("bar");
+    EXPECT_STREQ("F:\\bar", fs_windows.GetCString());
+    EXPECT_STREQ("F:", fs_windows.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString());
+
+    FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix);
+    fs_posix_root.AppendPathComponent("bar");
+    EXPECT_STREQ("/bar", fs_posix_root.GetCString());
+    EXPECT_STREQ("/", fs_posix_root.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_posix_root.GetFilename().GetCString());
+
+    FileSpec fs_windows_root("F:", false, FileSpec::ePathSyntaxWindows);
+    fs_windows_root.AppendPathComponent("bar");
+    EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
+    EXPECT_STREQ("F:", fs_windows_root.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs_windows_root.GetFilename().GetCString());
+}
+
+TEST(FileSpecTest, CopyByAppendingPathComponent)
+{
+    FileSpec fs = FileSpec("/foo", false, FileSpec::ePathSyntaxPosix).CopyByAppendingPathComponent("bar");
+    EXPECT_STREQ("/foo/bar", fs.GetCString());
+    EXPECT_STREQ("/foo", fs.GetDirectory().GetCString());
+    EXPECT_STREQ("bar", fs.GetFilename().GetCString());
+}