Message ID | 1434775646-2625-1-git-send-email-martin.galvan@tallertechnologies.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 43323 invoked by alias); 20 Jun 2015 04:47:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 43311 invoked by uid 89); 20 Jun 2015 04:47:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qk0-f172.google.com Received: from mail-qk0-f172.google.com (HELO mail-qk0-f172.google.com) (209.85.220.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 20 Jun 2015 04:47:35 +0000 Received: by qkhu186 with SMTP id u186so71358441qkh.0 for <gdb-patches@sourceware.org>; Fri, 19 Jun 2015 21:47:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=bQjgBurQTZFg+z80O+Pk8kpjXWRaOZIyRg95fwlOpP0=; b=FyAFKYp0jr6qk6HwWZduIvnhY7WIkQURgdDJeXVkSBN39dvoyZTpVHgnXsWtFTu08Z vNSZG37kxGYkHQQgNNqtExcJ2kz//JkiDcO0aTzCQJ+HtwtPDQj4LgWifvweAYlt/Ig3 dkZNJ/b6Ydobd9FwPJBFmvLITvLsIM98PYABYoXDYgvw5EANUn4+tztatl0jIAPHR0C/ 8/96TJL3jzZjcUKSGBuPQksEd4xwYvIzQlxXoS7Ml1UZnG5FJQf1cDUmosJK2c0LuLUv AiWfBPQcQ5uUnWs/y8mu8GgrBL/rPTKXnKKRfJdQiA1jxmdLDSi+hnoHy5qNqQZUom+V pb7g== X-Gm-Message-State: ALoCoQlMZJm6fGngo3Uoghn7Kh7Rqkc5px14XMCtXlK3UyEU5mUim/MYHQ+6tIV9ypVzBu4DbcVf X-Received: by 10.140.216.18 with SMTP id m18mr27319824qhb.19.1434775652953; Fri, 19 Jun 2015 21:47:32 -0700 (PDT) Received: from localhost.localdomain ([181.31.101.84]) by mx.google.com with ESMTPSA id g17sm6801384qkh.18.2015.06.19.21.47.31 for <gdb-patches@sourceware.org> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 19 Jun 2015 21:47:32 -0700 (PDT) From: Martin Galvan <martin.galvan@tallertechnologies.com> To: gdb-patches@sourceware.org Subject: [PING][PATCH] testsuite: Escape a loose '[' character inside a regexp. Date: Sat, 20 Jun 2015 01:47:26 -0300 Message-Id: <1434775646-2625-1-git-send-email-martin.galvan@tallertechnologies.com> |
Commit Message
Martin Galvan
June 20, 2015, 4:47 a.m. UTC
Seems like an obvious fix to me, but I wanted to check just in case. Ok to commit? --- gdb/testsuite/ChangeLog | 4 ++++ gdb/testsuite/lib/gdb.exp | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
Comments
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 ']')?
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?
On Tue, Jun 23, 2015 at 12:50 PM, Doug Evans <dje@google.com> wrote: > 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? If Yao thinks it's ok then I can send a v2 including that comment.
On Tue, Jun 23, 2015 at 10:50 AM, Doug Evans <dje@google.com> wrote: > 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. Actually, this is arguably s.o.p. Bleah. Otherwise \r\n inside {} won't get expanded as CR,LF. E.g. compare set foo {a\r\nb} vs set foo "a\r\nb" That \[ gets expanded as [ is no different than \z getting expanded as z. % set foo "a\zb\[c" azb[c % Still a bit confusing since \[ vs [ different in most contexts in Tcl. So I'd still advocate for a comment.
Doug Evans <dje@google.com> writes: > 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 [ == \[. [ and \[ may be used everywhere in the testsuite, so I don't know how much useful that we add comment that [ == \[ for gdb_test. Nobody will realise such difference is documented in the gdb_test comment. I agree that it is impractical to change every \[ to [, but in this patch, I am inclined to do the change in the reversed direction, which is to change \[ to [ within proc test_class_help.
On Tue, Jun 23, 2015 at 11:28 AM, Yao Qi <qiyaoltc@gmail.com> wrote: > Doug Evans <dje@google.com> writes: > >> 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 [ == \[. > > [ and \[ may be used everywhere in the testsuite, so I don't know how > much useful that we add comment that [ == \[ for gdb_test. Nobody will > realise such difference is documented in the gdb_test comment. That's not a reason to not document it at all. If I can't something to the code I'll add a note to the wiki. > I agree that it is impractical to change every \[ to [, but in this > patch, I am inclined to do the change in the reversed direction, which > is to change \[ to [ within proc test_class_help. Fine by me.
Doug Evans <dje@google.com> writes: > Actually, this is arguably s.o.p. Bleah. > Otherwise \r\n inside {} won't get expanded as CR,LF. \r\n are also special for tcl regexps. So it doesn't matter whether they are expanded by the tcl parser or the regexp engine. Andreas.
On Tue, Jun 23, 2015 at 12:04 PM, Andreas Schwab <schwab@linux-m68k.org> wrote: > Doug Evans <dje@google.com> writes: > >> Actually, this is arguably s.o.p. Bleah. >> Otherwise \r\n inside {} won't get expanded as CR,LF. > > \r\n are also special for tcl regexps. So it doesn't matter whether > they are expanded by the tcl parser or the regexp engine. Sorry, yeah. http://tcl.tk/man/tcl8.5/TclCmd/re_syntax.htm
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index e01f883..95338b3 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2015-06-13 Martin Galvan <martin.galvan@tallertechnologies.com> + + * lib/gdb.exp (test_class_help): Escape a loose '['. + 2015-06-12 Antoine Tremblay <antoine.tremblay@ericsson.com> PR breakpoints/16465 diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 41797e7..2be9ba4 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -4674,7 +4674,7 @@ proc test_class_help { command_class expected_initial_lines args } { set l_stock_body { "List of commands\:.*\[\r\n\]+" "Type \"help\" followed by command name for full documentation\.\[\r\n\]+" - "Type \"apropos word\" to search for commands related to \"word\"\.[\r\n\]+" + "Type \"apropos word\" to search for commands related to \"word\"\.\[\r\n\]+" "Command name abbreviations are allowed if unambiguous\." } set l_entire_body [concat $expected_initial_lines $l_stock_body]