From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70103 invoked by alias); 26 Jan 2016 16:56:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 69806 invoked by uid 89); 26 Jan 2016 16:56:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=violations, permissive, documentary, Hx-languages-length:2490 X-HELO: mail-io0-f169.google.com Received: from mail-io0-f169.google.com (HELO mail-io0-f169.google.com) (209.85.223.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 26 Jan 2016 16:56:30 +0000 Received: by mail-io0-f169.google.com with SMTP id q21so192439613iod.0 for ; Tue, 26 Jan 2016 08:56:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=DPD10TpS40blSMCn1Mp13MzGU80OxKzI1uCzk/LGbek=; b=izObsPmuoJQb7BibNsa2I6q+LnoCvykQF0fEcysDO1VfeZVJNyYqvvkyUis1I75Cna ZudHr7ypbKAPrbkS2PBO1YHm/ZmnwbrkQZAkcEfmaTtXBtmHQ54djtK0kvHRjDnH3Cgo eEuVVzulXaR+aRg3Z6GgajMKfMA9R1W4sOr3ALM/24YMRz9MmY4okCFtMqQxqAk49SHK cgWlb7TtE+67QW11g8c/Z12ouOyNSgSVzo201zTSw/9EpC6Vbs8G1JhMfzMLWrrPotsN NdMNakQ2laELq5HIIR2BkDXg/rqQlTh1bSnlhqqD8Pd5/fodvnpS0cDlKeQFrc49z34j JQVg== X-Gm-Message-State: AG10YOR45D//Wh4rmwpRBUIDuRtb8gT/M74af0Rv7bpsyYDHqdO35C6qxObg6ZNfNjDMh7ZTXlWu2Lsk+QVfgcpd X-Received: by 10.107.12.13 with SMTP id w13mr22204250ioi.33.1453827387609; Tue, 26 Jan 2016 08:56:27 -0800 (PST) MIME-Version: 1.0 Received: by 10.64.41.225 with HTTP; Tue, 26 Jan 2016 08:55:47 -0800 (PST) In-Reply-To: <56A65AD6.8060001@codesourcery.com> References: <047d7b6d967a74c69a0529168b2e@google.com> <56954538.1030007@redhat.com> <56A65AD6.8060001@codesourcery.com> From: Doug Evans Date: Tue, 26 Jan 2016 16:56:00 -0000 Message-ID: Subject: Re: [PING] Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303 To: Don Breazeal Cc: Keith Seitz , "gdb-patches@sourceware.org ml" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-01/txt/msg00659.txt.bz2 On Mon, Jan 25, 2016 at 9:26 AM, Don Breazeal wrote: > On 1/12/2016 10:26 AM, Keith Seitz wrote: >> On 01/11/2016 02:34 PM, Doug Evans wrote: >>> > - a complete test, just cheap and documentary. */ >>> > - if (strchr (name, '<') == NULL && strchr (name, '(') == NULL) >>> > - gdb_assert (strchr (name, ':') == NULL); >>> > - >>> >>> Heya. >>> >>> The assert is intended to catch (some) violations of this >>> (from the function comment): >>> >>> NAME is guaranteed to not have any scope (no "::") in its name, though >>> if for example NAME is a template spec then "::" may appear in the >>> argument list. >> [snip] >>> On that I'm kinda ambivalent, but I like having the assert >>> watch for the stated invariant. >>> >>> Thoughts? >> >> I missed that comment. [Well, I didn't even look at it. I'm so used to >> seeing no/minimal comments for symbol searching functions that I seldom >> even look for them. My bad.] >> >> That seems like a reasonable assertion, then, as long as it really does >> test what it is supposed to. How about: >> >> if (strchr (name, '<') == NULL && strchr (name, '(') == NULL) >> gdb_assert (strstr (name, "::") == NULL); >> >> Or something like that? >> >>> > diff --git a/gdb/cp-support.c b/gdb/cp-support.c >>> > index df127c4..a71c6ad 100644 >>> > --- a/gdb/cp-support.c >>> > +++ b/gdb/cp-support.c >>> > @@ -1037,8 +1037,13 @@ cp_find_first_component_aux (const char *name, >>> > int permissive) >>> > return strlen (name); >>> > } >>> > case '\0': >>> > - case ':': >>> > return index; >>> > + case ':': >>> > + /* ':' marks a component iff the next character is also a ':'. >>> > + Otherwise it is probably malformed input. */ >>> > + if (name[index + 1] == ':') >>> > + return index; >>> > + break; >>> >>> What if name[index+2] is also ':'? :-) >>> >> >> I don't think that matters at all. It isn't the scope operator in C++ >> unless it is *two* colons. Not just a single colon. [Note that I believe >> we are going to have to deal with the general single-colon issue when >> running this code with abitags, but that's a patch for some other time. >> Or maybe this patch already mitigates that to a degree. I haven't >> checked into it at all.] >> >> Keith >> > > Hi Doug, any thoughts on earlier responses from Keith and me to your > comments on this issue? > Thanks > --Don Sorry, nothing more to add. Keith's suggestion is fine by me.