[Lldb-commits] [PATCH] Don't remove backslashes from arguments unless the following char is recognized.

[Zachary Turner]

Hi jingham, clayborg,

This fixes file paths on Windows, as you can now write, for example, d:\foo\bar.txt, but does not break the case that this tokenization logic was originally designed for, which is to allow escaping of things like quotes and double quotes, so that all of the escapable characters can appear in the same string together.

I think this is a much simpler and more straightforward fix than adding a different tokenizer.  Having only 1 tokenizer means we don't have to worry in the future about tests breaking because someone wrote their test on unix and tried to execute a command with a syntax that wouldn't work on Windows, etc.  So hopefully we can reach an agreement about this fix.

http://reviews.llvm.org/D7018

Files:
  source/Interpreter/Args.cpp

Index: source/Interpreter/Args.cpp
===================================================================
--- source/Interpreter/Args.cpp
+++ source/Interpreter/Args.cpp
@@ -165,7 +165,7 @@
     if (command && command[0])
     {
         static const char *k_space_separators = " \t";
-        static const char *k_space_separators_with_slash_and_quotes = " \t \\'\"";
+        static const char *k_escapable_characters = " \t\\'\"";
         const char *arg_end = nullptr;
         const char *arg_pos;
         for (arg_pos = command;
@@ -201,7 +201,7 @@
 
             do
             {
-                arg_end = ::strcspn (arg_pos, k_space_separators_with_slash_and_quotes) + arg_pos;
+                arg_end = ::strcspn (arg_pos, k_escapable_characters) + arg_pos;
 
                 switch (arg_end[0])
                 {
@@ -215,7 +215,6 @@
                         arg.append (arg_piece_start);
                     arg_complete = true;
                     break;
-                    
                 case '\\':
                     // Backslash character
                     switch (arg_end[1])
@@ -227,22 +226,21 @@
                             break;
 
                         default:
-                            if (quote_char == '\0')
+                            // Only consider this two-character sequence an escape sequence if we're unquoted and
+                            // the character after the backslash is a whitelisted escapable character.  Otherwise
+                            // leave the character sequence untouched.
+                            if (quote_char == '\0' && (nullptr != strchr(k_escapable_characters, arg_end[1])))
                             {
                                 arg.append (arg_piece_start, arg_end - arg_piece_start);
-                                if (arg_end[1] != '\0')
-                                {
-                                    arg.append (arg_end + 1, 1);
-                                    arg_pos = arg_end + 2;
-                                    arg_piece_start = arg_pos;
-                                }
+                                arg.append (arg_end + 1, 1);
+                                arg_pos = arg_end + 2;
+                                arg_piece_start = arg_pos;
                             }
                             else
                                 arg_pos = arg_end + 2;
                             break;
                     }
                     break;
-                
                 case '"':
                 case '\'':
                 case '`':

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7018.18309.patch
Type: text/x-patch
Size: 2592 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150116/57cd09b1/attachment.bin>