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

[Keith Seitz <keiths at redhat dot com>]

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