This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFAv2 1/6] New cli-utils.h/.c function extract_info_print_args


>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Tom> I tend to think this should go somewhere else, since cli-utils is more
Tom> like "low level" parsing helpers.  But I don't really know where I
Tom> guess.

Philippe> Yes, I found no better (existing) place, so I kept it there.
Philippe> If you prefer, I can create cli-arg_utils.h for 'higher level'
Philippe> arg parsing helpers if you believe this is cleaner.

I think your current approach is fine.

Philippe> I looked at using gdb_argv, but concluded that using gdb_argv
Philippe> would be backward incompatible for some likely use cases.
[...]
Philippe> So, I did not switch fully to gdb_argv, but made
Philippe> extract_arg_maybe_quoted quoting and escaping compatible
Philippe> with gdb_argv.

Thank you for looking into this.

Tom> One question I have is whether we could just change extract_arg to do
Tom> the dequoting.  Auditing the existing calls to see if this change would
Tom> negatively impact them might not be too hard.  And, it would be nice to
Tom> make gdb's CLI a bit more regular.

Philippe> I examined the calls to extract_arg : I found one possible unlikely
Philippe> backward incompatibility related to single quote handling :
Philippe> the mem command accepts expressions for addresses, and expressions
Philippe> in Ada can contain single quotes.

C-ish languages use single quotes for char constants, so this would be
pretty bad there too.

The "mem" command got the parsing wrong.  The incorrectness dates back
to when it was introduced in 2001.  For example (today's code but it
hasn't substantially changed):

  std::string tok = extract_arg (&args);
  if (tok == "")
    error (_("no lo address"));
  lo = parse_and_eval_address (tok.c_str ());

This is user-hostile for non-trivial expressions, because it forces you
to write them without spaces.

Instead code like this should use parse_to_comma_and_eval.

Maybe it is too late to fix the "mem" command.  I am not sure.  I do
know that back when I approved a similar change to "disasssemble" (I
thought I'd written that!  But the history shows not), there were
complaints -- we broke people's workarounds for the bad parsing
behavior.  On the other hand, "disasssemble" is probably used a lot more
than "mem".

Philippe> So, it looks possible to have extract_arg behaviour replaced by
Philippe> extract_arg_maybe_quoted behaviour.
Philippe> If you agree with the above analysis, I will work on that in a separate
Philippe> patch series.

One idea might be to upgrade the calls where it seems reasonable and
then leave the legacy behavior for the ones where it is not... perhaps
at the end, renaming extract_arg so that the name makes it clear that it
shouldn't be used in new code.

What do you think of that?  It's not necessary for you to do all the
work involved.

Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]