This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PING][PATCH] testsuite: Escape a loose '[' character inside a regexp.
- From: Doug Evans <dje at google dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: Martin Galvan <martin dot galvan at tallertechnologies dot com>, gdb-patches <gdb-patches at sourceware dot org>
- Date: Tue, 23 Jun 2015 10:50:51 -0500
- Subject: Re: [PING][PATCH] testsuite: Escape a loose '[' character inside a regexp.
- Authentication-results: sourceware.org; auth=none
- References: <1434775646-2625-1-git-send-email-martin dot galvan at tallertechnologies dot com> <86bng8rtva dot fsf at gmail dot com>
On Mon, Jun 22, 2015 at 4:10 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> Martin Galvan <martin.galvan@tallertechnologies.com> writes:
>
>> Seems like an obvious fix to me, but I wanted to check just in case.
>> Ok to commit?
>
> Looks the code works well without this patch. Do we really need to
> escape '[' (and ']')?
In that case there's still something wrong, or at least still
something requiring patching.
When code isn't consistent readers start wondering if there's a problem,
and may decide to look into it.
And if there isn't a problem then it's just wasted time.
Here the inconsistency happens because it turns out that
in this context [ and \[ have the same effect.
[I added some printfs to gdb_test to find this out.]
I think that's a bug but I don't know what would be involved
in fixing it. OTOH, we should at least make the code consistent.
Generally [ is escaped to avoid Tcl treating it as a subexpression to evaluate.
But the escaping isn't needed if the string is wrapped in {} braces
which is the case here. Thus to me it feels like it's all the other
escaped brackets that need to be fixed (by changing \[ \] to [ ]).
OTOH, if one carried that through to completion it would
involve a *lot* of changes. help.exp is replete with them.
So for now I think the thing to do is apply this patch,
*and* add a comment somewhere (the function comment
for gdb_test?) documenting that [ == \[.
Thoughts?