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

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

Ping

Any suggestion?

--
Wei-cheng

On Fri, Jun 7, 2013 at 11:49 PM, Wei-cheng Wang <cole945@gmail.com> wrote:
> Hi,
>
> This is a patch for parsing quoted string for restore/dump commands.
> Test cases are also added.
>
> Without this fix, we cannot dump to or restore from a file in a path that
> has spaces.
>
> Before fixing.
>
> $ make check RUNTESTFLAGS="dump.exp" | grep "FAIL" | sort -u | uniq -c
>   1 FAIL: gdb.base/dump.exp: array as value, binary, escape whitespace; file
> restored ok
>   1 FAIL: gdb.base/dump.exp: array as value, binary, escape whitespace;
> value restored ok
>   1 FAIL: gdb.base/dump.exp: array as value, binary, quoted whitespace; file
> restored ok
>   1 FAIL: gdb.base/dump.exp: array as value, binary, quoted whitespace;
> value restored ok
>   1 FAIL: gdb.base/dump.exp: dump array as value, binary, escaped whitespace
>   1 FAIL: gdb.base/dump.exp: dump array as value, binary, quoted whitespace
>   1 FAIL: gdb.base/dump.exp: dump struct as value, binary, escaped
> whitespace
>   1 FAIL: gdb.base/dump.exp: dump struct as value, binary, quoted whitespace
>   1 FAIL: gdb.base/dump.exp: struct as value, binary, escape whitespace;
> file restored ok
>   1 FAIL: gdb.base/dump.exp: struct as value, binary, escape whitespace;
> value restored ok
>   1 FAIL: gdb.base/dump.exp: struct as value, binary, quoted whitespace;
> file restored ok
>   1 FAIL: gdb.base/dump.exp: struct as value, binary, quoted whitespace;
> value restored ok
>
> After fixing.
>
> # of expected passes            199
>
>
> ---
>  gdb/ChangeLog                   |    5 ++++
>  gdb/cli/cli-dump.c              |   52
> ++++++++++++++++++++++++++++++++++-----
>  gdb/testsuite/ChangeLog         |    4 +++
>  gdb/testsuite/gdb.base/dump.exp |   34 ++++++++++++++++++++++++-
>  4 files changed, 88 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 5fdb1f8..e0b1c53 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2013-06-07  Wei-cheng Wang  <cole945@gmail.com>
> +
> +       * cli/cli-dump.c (scan_filename_with_cleanup): Parse quoted or
> +       escaped string when scanning filename.
> +
>  2013-06-05  Doug Evans  <dje@google.com>
>             Keith Seitz  <keiths@redhat.com>
>
> diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
> index 208916c..1fc9f3f 100644
> --- a/gdb/cli/cli-dump.c
> +++ b/gdb/cli/cli-dump.c
> @@ -80,14 +80,54 @@ 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;
> +      int capacity;
> +
> +      inp = (*cmd) = 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 (*cmd, " \t") + 1;
> +      outp = filename = (char *) xmalloc (capacity);
> +
> +      /* Quoting character.  */
> +      if (*inp == '\'' || *inp == '"')
> +       quote = *inp++;
> +
> +      for (; *inp != '\0'; inp++)
> +       {
> +         /* Stop by whitespace, if not quoted or escaped. */
> +         if (whitespace (*inp) && !escape && !quote)
> +           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)
> +           {
> +             int len = outp - filename;
>
> -      (*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';
>        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 698f116..e28d75e 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2013-06-07  Wei-cheng Wang  <cole945@gmail.com>
> +
> +       * gdb.base/dump.exp: Test tests for filenames with spaces.
> +
>  2013-06-06  Doug Evans  <dje@google.com>
>
>         * gdb.cp/derivation.exp: Make tests have unique names.
> 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"
> --