This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Fwd: Re: Patch to support spaces in filenames & paths]
- From: Tom Tromey <tromey at redhat dot com>
- To: Denis PILAT <denis dot pilat at st dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Mon, 27 Apr 2009 15:39:36 -0600
- Subject: Re: [Fwd: Re: Patch to support spaces in filenames & paths]
- References: <4981BA17.2090501@st.com>
- Reply-to: tromey at redhat dot com
>>>>> "Denis" == Denis PILAT <denis.pilat@st.com> writes:
Oops, I mistyped and sent a draft before I had finished.
This is the same note with one minor addition at the end.
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> + argv = gdb_buildargv (args);
Denis> + make_cleanup_freeargv (argv);
You have to save the first cleanup in a function and then either "do"
or "discard" it before any return.
Tom