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

Wei-cheng Wang
2013-06-15T06:46:16Z

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"
> --