[PING] Re: [PATCH] Fix problem handling colon in linespec, PR breakpoints/18303

[Don Breazeal <donb@codesourcery.com>]

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