Re: [Fwd: Re: Patch to support spaces in filenames & paths]

[Tom Tromey <tromey@redhat.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