[Fwd: Re: Patch to support spaces in filenames & paths]
Tom Tromey
tromey@redhat.com
Mon Apr 27 21:38:00 GMT 2009
>>>>> "Denis" == Denis PILAT <denis.pilat@st.com> writes:
Denis> Ping !
Sorry for the delay on this.
I read both the patches for this issue, plus the thread on the mailing
list.
Denis> I read your comments and produce a new patch that uses gdb_buildargv
Denis> (as Pedro suggested), inspired from add_symbol_file_command as well.
I don't like this approach.
With the current patch, we will accept a quoted expression. This
seems very ugly to me. I wouldn't be surprised if completion failed
in this case. And, given that I would not support a new command using
a quoting scheme like this, it seems odd to allow one via refactoring.
I think this problem can be addressed to some degree by more carefully
parsing the address arguments.
Second, concatenating the final arguments using a space is not
completely correct, because it does not preserve the meaning of all
possible expressions. This is a pedantic point, since such
expressions are unlikely to be used here.
In the long term, I would like to see gdb move toward a more uniform
approach to parsing its arguments, and to allowing full expressions in
more places. I don't think gdb_buildargv can be the foundation for
that, though.
Since there only seems to be a single argument that has a problem
here, how about what Daniel suggested: a function that uses the same
quoting rules as gdb_buildargv, but which only peels off a single
argument?
A few notes on the patch itself follow.
Denis> I also removed lot of dead code in cli/cli-dump.c and
Denis> fopen_with_cleanup is now used only locally, so removed from
Denis> cli-dump.h.
In this case, make the function static as well.
Denis> Dump restore and append commands are impacted by this patch.
I think this may need a documentation update.
Denis> @@ -559,8 +515,14 @@ restore_command (char *args, int from_tt
Denis> char *filename;
Denis> struct callback_data data;
Denis> bfd *ibfd;
Denis> - int binary_flag = 0;
Denis> -
Denis> + char **argv;
Denis> + int binary_flag;
Denis> + char *arg;
Denis> + int arg_num;
Denis> + char *end_addr = NULL;
Denis> + filename = NULL;
Denis> + binary_flag = 0;
Denis> +
Denis> if (!target_has_execution)
Denis> noprocess ();
Denis> @@ -568,43 +530,54 @@ restore_command (char *args, int from_tt
Denis> data.load_start = 0;
Denis> data.load_end = 0;
Denis> - /* Parse the input arguments. First is filename (required). */
Denis> - filename = scan_filename_with_cleanup (&args, NULL);
Denis> - if (args != NULL && *args != '\0')
Denis> - {
Denis> - char *binary_string = "binary";
Denis> + argv = gdb_buildargv (args);
Denis> + make_cleanup_freeargv (argv);
Denis> - /* Look for optional "binary" flag. */
Denis> - if (strncmp (args, binary_string, strlen (binary_string)) == 0)
Denis> - {
Denis> - binary_flag = 1;
Denis> - args += strlen (binary_string);
Denis> - args = skip_spaces (args);
Denis> - }
Denis> - /* Parse offset (optional). */
Denis> - if (args != NULL && *args != '\0')
Denis> - data.load_offset =
Denis> - parse_and_eval_long (scan_expression_with_cleanup (&args, NULL));
Denis> - if (args != NULL && *args != '\0')
Denis> - {
Denis> - /* Parse start address (optional). */
Denis> - data.load_start =
Denis> - parse_and_eval_long (scan_expression_with_cleanup (&args, NULL));
Denis> - if (args != NULL && *args != '\0')
Denis> - {
Denis> - /* Parse end address (optional). */
Denis> - data.load_end = parse_and_eval_long (args);
Denis> - if (data.load_end <= data.load_start)
Denis> - error (_("Start must be less than end."));
Denis> - }
Denis> - }
Denis> + /* At least filename argument is required. */
Denis> + if (argv == NULL || argv[0] == NULL)
Denis> + error (_("Missing filename as the first argument."));
Denis> +
Denis> + /* The first argument is the file name. */
Denis> + filename = tilde_expand (argv[0]);
Denis> + make_cleanup (xfree, filename);
Denis> +
Denis> + /* Look for optionals arguments from here: "binary", offset,
Denis> + start and end address. */
Denis> + if (argv[1] != NULL && strcmp (argv[1], "binary") == 0)
Denis> + binary_flag = 1;
Denis> +
Denis> + arg_num = 1 + binary_flag;
Denis> + for (arg = argv[arg_num]; arg != NULL; arg = argv[++arg_num])
Denis> + {
Denis> + /* parse offset argument (optional). */
Denis> + if (arg_num == 1 + binary_flag)
Denis> + data.load_offset = parse_and_eval_long (arg);
Denis> +
Denis> + /* parse start address argument (optional). */
Denis> + else if (arg_num == 2 + binary_flag)
Denis> + data.load_start = parse_and_eval_long (arg);
Denis> +
Denis> + /* Rest of line is considered as end address optional argument. */
Denis> + else if (arg_num == 3 + binary_flag )
Denis> + end_addr = arg;
Denis> + else if (arg_num > 3 + binary_flag)
Denis> + end_addr = concat (end_addr, " ", arg, (char *)NULL);
Denis> + }
Denis> +
Denis> + /* If end address argument is given, parse it and test it's greater than
Denis> + the start address. */
Denis> + if (end_addr != NULL)
Denis> + {
Denis> + data.load_end = parse_and_eval_long (end_addr);
Denis> + if (data.load_end <= data.load_start)
Denis> + error (_("Start must be less than end."));
Denis> }
Denis> if (info_verbose)
Denis> printf_filtered ("Restore file %s offset 0x%lx start 0x%lx end 0x%lx\n",
Denis> - filename, (unsigned long) data.load_offset,
Denis> - (unsigned long) data.load_start,
Denis> - (unsigned long) data.load_end);
Denis> + filename, (unsigned long) data.load_offset,
Denis> + (unsigned long) data.load_start,
Denis> + (unsigned long) data.load_end);
Denis> if (binary_flag)
Denis> {
Denis> @@ -775,7 +748,8 @@ to the specified FILE in raw target orde
Denis> c = add_com ("restore", class_vars, restore_command, _("\
Denis> Restore the contents of FILE to target memory.\n\
Denis> -Arguments are FILE OFFSET START END where all except FILE are optional.\n\
Denis> +Arguments are FILE \"binary\" OFFSET START END\n\
Denis> +where all except FILE are optional.\n\
Denis> OFFSET will be added to the base address of the file (default zero).\n\
Denis> If START and END are given, only the file contents within that range\n\
Denis> (file relative) will be restored to target memory."));
Denis> Index: cli/cli-dump.h
Denis> ===================================================================
Denis> RCS file: /cvs/src/src/gdb/cli/cli-dump.h,v
Denis> retrieving revision 1.7
Denis> diff -u -p -r1.7 cli-dump.h
Denis> --- cli/cli-dump.h 3 Jan 2009 05:57:54 -0000 1.7
Denis> +++ cli/cli-dump.h 5 Jan 2009 14:14:36 -0000
Denis> @@ -24,15 +24,4 @@ extern void add_dump_command (char *name
Denis> void (*func) (char *args, char *mode),
Denis> char *descr);
Denis> -/* Utilities for doing the dump. */
Denis> -extern char *scan_filename_with_cleanup (char **cmd, const char *defname);
Denis> -
Denis> -extern char *scan_expression_with_cleanup (char **cmd, const char *defname);
Denis> -
Denis> -extern FILE *fopen_with_cleanup (const char *filename, const char *mode);
Denis> -
Denis> -extern char *skip_spaces (char *inp);
Denis> -
Denis> -extern struct value *parse_and_eval_with_error (char *exp, const char *fmt, ...) ATTR_FORMAT (printf, 2, 3);
Denis> -
Denis> #endif
Denis> 2009-01-05 Denis Pilat <denis.pilat@st.com>
Denis> * gdb.base/help.exp: add "binary" option for restore command.
Denis> Index: gdb.base/help.exp
Denis> ===================================================================
Denis> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
Denis> retrieving revision 1.31
Denis> diff -u -p -r1.31 help.exp
Denis> --- gdb.base/help.exp 16 Nov 2008 18:03:25 -0000 1.31
Denis> +++ gdb.base/help.exp 5 Jan 2009 14:02:44 -0000
Denis> @@ -355,7 +355,7 @@ gdb_test "help run" "Start debugged prog
Denis> # test help rbreak
Denis> gdb_test "help rbreak" "Set a breakpoint for all functions matching REGEXP\." "help rbreak"
Denis> # test help restore
Denis> -gdb_test "help restore" "Restore the contents of FILE to target memory\.\[\r\n\]+Arguments are FILE OFFSET START END where all except FILE are optional\.\[\r\n\]+OFFSET will be added to the base address of the file \\(default zero\\)\.\[\r\n\]+If START and END are given, only the file contents within that range\[\r\n\]+\\(file relative\\) will be restored to target memory\."
Denis> +gdb_test "help restore" "Restore the contents of FILE to target memory\.\[\r\n\]+Arguments are FILE \"binary\" OFFSET START END\[\r\n\]+where all except FILE are optional\.\[\r\n\]+OFFSET will be added to the base address of the file \\(default zero\\)\.\[\r\n\]+If START and END are given, only the file contents within that range\[\r\n\]+\\(file relative\\) will be restored to target memory\."
Denis> # test help return
Denis> gdb_test "help return" "Make selected stack frame return to its caller\.\[\r\n\]+Control remains in the debugger, but when you continue\[\r\n\]+execution will resume in the frame above the one now selected\.\[\r\n\]+If an argument is given, it is an expression for the value to return\." "help return"
Denis> # test help reverse-search
Denis> ----------
Tom
More information about the Gdb-patches
mailing list