From: Tom Tromey <tom@tromey.com>
To: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Cc: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Subject: Re: [RFAv2 1/6] New cli-utils.h/.c function extract_info_print_args
Date: Mon, 24 Sep 2018 13:07:00 -0000 [thread overview]
Message-ID: <87tvmfniwi.fsf@tromey.com> (raw)
In-Reply-To: <1537733790.3924.13.camel@skynet.be> (Philippe Waroquiers's message of "Sun, 23 Sep 2018 22:16:30 +0200")
>>>>> "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
next prev parent reply other threads:[~2018-09-24 13:07 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-26 16:54 [RFAv2 0/6] info [args|functions|locals|variables] [-q] [-t TYPEREGEXP] [NAMEREGEXP] Philippe Waroquiers
2018-08-26 16:54 ` [RFAv2 5/6] Announce changes in NEWS to info [args|functions|locals|variables] Philippe Waroquiers
2018-08-26 18:21 ` Eli Zaretskii
2018-08-27 4:39 ` Philippe Waroquiers
2018-08-27 14:59 ` Eli Zaretskii
2018-08-26 16:54 ` [RFAv2 4/6] Document changes " Philippe Waroquiers
2018-08-26 18:19 ` Eli Zaretskii
2018-08-26 16:54 ` [RFAv2 2/6] Make struct type_print_options default_ptype_flags non static Philippe Waroquiers
2018-08-26 16:54 ` [RFAv2 1/6] New cli-utils.h/.c function extract_info_print_args Philippe Waroquiers
2018-09-18 15:31 ` Tom Tromey
2018-09-23 20:16 ` Philippe Waroquiers
2018-09-24 13:07 ` Tom Tromey [this message]
2018-09-25 4:37 ` Philippe Waroquiers
2018-08-26 16:54 ` [RFAv2 6/6] Add a test case for info args|functions|locals|variables [-q] [-t TYPEREGEXP] [NAMEREGEXP] Philippe Waroquiers
2018-08-26 16:54 ` [RFAv2 3/6] Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables] Philippe Waroquiers
2018-09-18 15:52 ` Tom Tromey
2018-09-23 21:34 ` Philippe Waroquiers
2018-09-06 20:16 ` PING Re: [RFAv2 0/6] info [args|functions|locals|variables] [-q] [-t TYPEREGEXP] [NAMEREGEXP] Philippe Waroquiers
2018-09-13 19:27 ` PING^2 " Philippe Waroquiers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tvmfniwi.fsf@tromey.com \
--to=tom@tromey.com \
--cc=gdb-patches@sourceware.org \
--cc=philippe.waroquiers@skynet.be \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox