[PATCH] Fix hardcoded path seperator in two code locations

Yaron Keren yaron.keren at gmail.com
Fri May 9 02:32:39 PDT 2014


Hi echristo, rnk, chapuni,

Earlier duscussion:
 https://www.mail-archive.com/cfe-commits@cs.uiuc.edu/msg92891.html

Most of LLVM code uses the llvm::sys::path::append function to concatenate paths and will get the correct native slash. In some code locations it seems that direct string concatenation was used, possibly because the code writer knows there are no extra path sepeartors to deal with, so string concat was more efficient than append.

In the locations below, the path seperator is hardcoded as '/'. Fixed by exposing the native seperator from llvm::sys:;path and using it. 

As an alternative, the append function could be used but it's more code and less efficient.

http://reviews.llvm.org/D3687

Files:
  include/llvm/Support/Path.h
  lib/MC/MCDwarf.cpp
  lib/Support/Path.cpp
  lib/Support/SourceMgr.cpp

Index: include/llvm/Support/Path.h
===================================================================
--- include/llvm/Support/Path.h
+++ include/llvm/Support/Path.h
@@ -295,6 +295,11 @@
 /// @result true if \a value is a path separator character on the host OS
 bool is_separator(char value);
 
+/// @brief Return the preferred separator for this platform.
+///
+/// @result const char* to the preferred separator, null-terminated.
+const char *get_separator();
+
 /// @brief Get the typical temporary directory for the system, e.g., 
 /// "/var/tmp" or "C:/TEMP"
 ///
Index: lib/Support/Path.cpp
===================================================================
--- lib/Support/Path.cpp
+++ lib/Support/Path.cpp
@@ -569,6 +569,12 @@
   }
 }
 
+static const char preferred_separator_string[] = { preferred_separator, '\0' };
+
+const char *get_separator() {
+  return preferred_separator_string;
+}
+
 void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result) {
   result.clear();
 
Index: lib/MC/MCDwarf.cpp
===================================================================
--- lib/MC/MCDwarf.cpp
+++ lib/MC/MCDwarf.cpp
@@ -689,7 +689,7 @@
   const SmallVectorImpl<std::string> &MCDwarfDirs = context.getMCDwarfDirs();
   if (MCDwarfDirs.size() > 0) {
     MCOS->EmitBytes(MCDwarfDirs[0]);
-    MCOS->EmitBytes("/");
+    MCOS->EmitBytes(sys::path::get_separator());
   }
   const SmallVectorImpl<MCDwarfFile> &MCDwarfFiles =
     MCOS->getContext().getMCDwarfFiles();
Index: lib/Support/SourceMgr.cpp
===================================================================
--- lib/Support/SourceMgr.cpp
+++ lib/Support/SourceMgr.cpp
@@ -18,6 +18,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Locale.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
 using namespace llvm;
@@ -60,7 +61,7 @@
 
   // If the file didn't exist directly, see if it's in an include path.
   for (unsigned i = 0, e = IncludeDirectories.size(); i != e && !NewBuf; ++i) {
-    IncludedFile = IncludeDirectories[i] + "/" + Filename;
+    IncludedFile = IncludeDirectories[i] + sys::path::get_separator() + Filename;
     MemoryBuffer::getFile(IncludedFile.c_str(), NewBuf);
   }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3687.9239.patch
Type: text/x-patch
Size: 2295 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140509/5f1f12de/attachment.bin>


More information about the llvm-commits mailing list