Re: [PATCH] Fix for PR 15413 (segfault when completing "condition" for pending bp)

[Sergio Durigan Junior <sergiodj@redhat.com>]

Thanks for the review.

On Monday, May 06 2013, Pedro Alves wrote:

> On 05/06/2013 03:58 AM, Sergio Durigan Junior wrote:

>> +		xsnprintf (location, sizeof (location), "%d", b->number);
>> +	      }
>>  	    else
>> -	      xsnprintf (location, sizeof (location),  "%d.%d", b->number,
>> -			 count);
>> +	      {
>> +		if (b->loc->next == NULL)
>> +		  xsnprintf (location, sizeof (location), "%d", b->number);
>> +		else
>> +		  xsnprintf (location, sizeof (location),  "%d.%d", b->number,
>> +			     count);
>> +
>> +		loc = b->loc->next;
>
> This is always picking the same loc over and over?  I guess this means
> the test should be extended.  :-)

Ops, you're right, I will fix it right away, thanks for the catch.

>> +	      }
>>  
>>  	    if (strncmp (location, text, len) == 0)
>>  	      VEC_safe_push (char_ptr, result, xstrdup (location));
>>  
>>  	    ++count;
>>  	  }
>> +	while (loc != NULL);
>>        }
>>  
>
> I notice the condition completer is more broken than this, btw:
>
> $ ./gdb ./testsuite/gdb.cp/mb-ctor
> GNU gdb (GDB) 7.6.50.20130430-cvs
> (gdb) b Derived::Derived
> Breakpoint 1 at 0x400811: Derived::Derived. (2 locations)
> (gdb) info breakpoints
> Num     Type           Disp Enb Address            What
> 1       breakpoint     keep y   <MULTIPLE>
> 1.1                         y     0x0000000000400811 in Derived::Derived(int) at ../../../src/gdb/testsuite/gdb.cp/mb-ctor.cc:34
> 1.2                         y     0x0000000000400867 in Derived::Derived(int) at ../../../src/gdb/testsuite/gdb.cp/mb-ctor.cc:34
> (gdb) complete condition
> condition 1.1
> condition 1.2
> (gdb) complete condition 1
> condition 1.1
> condition 1.2
> (gdb) complete condition 1.
> condition 1.1.1
> condition 1.1.2
> (gdb) complete condition 1.1
> condition 1.1.1
> (gdb) complete condition 1.1
> condition 1.1.1
> (gdb) complete condition 1.1.1
> (gdb)
>
> Or:
>
> (gdb) condition<tab>
> (gdb) condition <tab>1.<tab>1.
> (gdb) condition 1.
> (gdb) condition 1.<tab>
> (gdb) condition 1.1.<tab>
> (gdb) condition 1.1.<enter>
> Bad breakpoint argument: '1.1.'
>
>
> BTW2, I'm thinking it'd make sense to always include the
> breakpoint-number-only ("%d", b->number) completion option, even if there
> are multiple locations?  That is, with breakpoint 1 having two locations,
> this would happen:
>
> (gdb) condition 1<tab>
> 1 1.1 1.2
>
> instead of:
> (gdb) condition 1<tab>
> (gdb) condition 1.
> (gdb) condition 1.<tab>
> 1.1 1.2
>
> Oh, wait, wait, wait...  The condition is a breakpoint property,
> not a location property, so what's with the completer suggesting
> location numbers at all?
>
> (gdb) info breakpoints
> Num     Type           Disp Enb Address            What
> 1       breakpoint     keep y   <MULTIPLE>
> 1.1                         y     0x0000000000400811 in Derived::Derived(int) at ../../../src/gdb/testsuite/gdb.cp/mb-ctor.cc:34
> 1.2                         y     0x0000000000400867 in Derived::Derived(int) at ../../../src/gdb/testsuite/gdb.cp/mb-ctor.cc:34
> (gdb) condition 1 0
> (gdb)
> (gdb) condition 1.2 0
> Bad breakpoint argument: '1.2 0'

So, if I understood your brain dump correctly, you're suggesting that
the "condition" command shouldn't complete multiple locations at all,
since the condition is inherent to the breakpoint, not to the
location(s).  Is that right?  I will submit a patch soon.

[OTOH, I guess it would make more sense if the condition were a location
property.]

Thanks,

-- 
Sergio