Re: [patch] Parse quoted string for restore/dump commands.

[Wei-cheng Wang <cole945 at gmail dot com>]

Hi Sergio,

Thank you for you review.
I've fixed them accordingly at the end of the mail.

On Sun, Jun 16, 2013 at 2:37 AM, Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
> On Friday, June 07 2013, Wei-cheng Wang wrote:
>
> Thanks for the patch, Wei-cheng.  I don't see your name on
> gdb/MAINTAINERS, do you have copyright assignment on file?  You will
> need one in order to commit the changes.  If you don't have it, please
> contact me off-list and I can send you the forms.

  We've signed the copryright assignment in GDB, on behalf of Andes Technology
  in Dec. 2009.  Is that sufficent?

>>
>> -      (*cmd) = skip_spaces (*cmd);
>> -      end = *cmd + strcspn (*cmd, " \t");
>> -      filename = savestring ((*cmd), end - (*cmd));
>> +           capacity *= 2;
>> +           filename = (char *) xrealloc (filename, capacity);
>> +           outp = filename + len;
>> +         }
>> +     }
>> +
>> +     *outp = '\0';
>
> Just for completeness, you could xrealloc (filename, outp - filename + 1)
> to save space.

  I was considering the trade-off between time and space.
  Assuming in most cases, there is no space in the path,
  the initial size of the buffer is the length to the first delimiter.
  If we expand the buffer only 1 more byte a time, xrealloc might be
  called much more times than double the size, so I use the same policy
  of buildargv.

  Any suggestion?

--
Wei-cheng

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7ae0797..a061f27 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2013-06-16  Wei-cheng Wang  <cole945@gmail.com>
+	    Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	* cli/cli-dump.c (scan_filename_with_cleanup): Parse quoted or
+	escaped string when scanning filename.
+
 2013-06-13  Doug Evans  <dje@google.com>

 	* dwarf2read.c (try_open_dwop_file): Work around behaviour of
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 208916c..06003a6 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -80,14 +80,55 @@ scan_filename_with_cleanup (char **cmd, const char *defname)
     }
   else
     {
-      /* FIXME: should parse a possibly quoted string.  */
-      char *end;
+      char *inp, *outp;
+      char quote = '\0';
+      int escape = 0;
+      size_t capacity;
+
+      inp = skip_spaces (*cmd);
+      /* Allocate initial buffer for filename.  If there is no space
+	 in quoted string, this should be the exactly size what we
+	 need.  */
+      capacity = strcspn (inp, " \t") + 1;
+      filename = xmalloc (capacity);
+      outp = filename;
+
+      /* Quoting character.  */
+      if (*inp == '\'' || *inp == '"')
+	quote = *inp++;
+
+      for (; *inp != '\0'; inp++)
+	{
+	  /* Stop by whitespace, if not quoted or escaped.  */
+	  if (isspace (*inp) && !escape && quote != '\0')
+	    break;
+
+	  if (escape)
+	    {
+	      escape = 0;
+	      *outp++ = *inp;
+	    }
+	  else if (*inp == '\\')
+	    escape = 1;
+	  else if (*inp == quote)
+	    quote = '\0';
+	  else
+	    *outp++ = *inp;
+
+	  /* Expand when needed.  */
+	  if (outp - filename >= capacity)
+	    {
+	      size_t len = outp - filename;

-      (*cmd) = skip_spaces (*cmd);
-      end = *cmd + strcspn (*cmd, " \t");
-      filename = savestring ((*cmd), end - (*cmd));
+	      capacity *= 2;
+	      filename = xrealloc (filename, capacity);
+	      outp = filename + len;
+	    }
+	}
+
+     *outp = '\0';
       make_cleanup (xfree, filename);
-      (*cmd) = skip_spaces (end);
+      (*cmd) = skip_spaces (inp);
     }
   gdb_assert (filename != NULL);

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 06217f9..77bc653 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2013-06-16  Wei-cheng Wang  <cole945@gmail.com>
+
+	* gdb.base/dump.exp: Test tests for filenames with spaces.
+
 2013-06-07  Pedro Alves  <palves@redhat.com>

 	* boards/native-extended-gdbserver.exp: Remove semicolon.
diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
index b2d0b3a..ab0481c 100644
--- a/gdb/testsuite/gdb.base/dump.exp
+++ b/gdb/testsuite/gdb.base/dump.exp
@@ -105,6 +105,18 @@ make_dump_file "dump bin val intarr1b.bin intarray" \
 make_dump_file "dump bin val intstr1b.bin intstruct" \
 	"dump struct as value, binary"

+make_dump_file "dump bin val \"intarr1c .bin\" intarray" \
+	"dump array as value, binary, quoted whitespace"
+
+make_dump_file "dump bin val \"intstr1c .bin\" intstruct" \
+	"dump struct as value, binary, quoted whitespace"
+
+make_dump_file "dump bin val intarr1d\\ .bin intarray" \
+	"dump array as value, binary, escaped whitespace"
+
+make_dump_file "dump bin val intstr1d\\ .bin intstruct" \
+	"dump struct as value, binary, escaped whitespace"
+
 make_dump_file "dump srec val intarr1.srec intarray" \
 	"dump array as value, srec"

@@ -300,6 +312,26 @@ test_restore_saved_value "intstr1.bin binary
$struct_start" \

 gdb_test "print zero_all ()" ".*"

+test_restore_saved_value "intarr1c\\ .bin binary $array_start" \
+	"array as value, binary, quoted whitespace" \
+	$array_val "intarray"
+
+test_restore_saved_value "intstr1c\\ .bin binary $struct_start" \
+	"struct as value, binary, quoted whitespace" \
+	$struct_val "intstruct"
+
+gdb_test "print zero_all ()" ".*"
+
+test_restore_saved_value "\"intarr1d .bin\" binary $array_start" \
+	"array as value, binary, escape whitespace" \
+	$array_val "intarray"
+
+test_restore_saved_value "\"intstr1d .bin\" binary $struct_start" \
+	"struct as value, binary, escape whitespace" \
+	$struct_val "intstruct"
+
+gdb_test "print zero_all ()" ".*"
+
 test_restore_saved_value "intarr2.bin binary $array_start" \
 	"array as memory, binary" \
 	$array_val "intarray"
@@ -505,4 +537,4 @@ if ![string compare $is64bitonly "no"] then {

 # clean up files

-remote_exec build "rm -f intarr1.bin intarr1b.bin intarr1.ihex
intarr1.srec intarr1.tekhex intarr2.bin intarr2b.bin intarr2.ihex
intarr2.srec intarr2.tekhex intstr1.bin intstr1b.bin intstr1.ihex
intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex
intstr2.srec intstr2.tekhex intarr3.srec"
+remote_exec build "rm -f intarr1.bin intarr1b.bin \"intarr1c .bin\"
\"intarr1d .bin\" intarr1.ihex intarr1.srec intarr1.tekhex intarr2.bin
intarr2b.bin intarr2.ihex intarr2.srec intarr2.tekhex intstr1.bin
intstr1b.bin \"intstr1c .bin\" \"intstr1d .bin\" intstr1.ihex
intstr1.srec intstr1.tekhex intstr2.bin intstr2b.bin intstr2.ihex
intstr2.srec intstr2.tekhex intarr3.srec"