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

[Pedro Alves <palves@redhat.com>]

On 05/06/2013 03:58 AM, Sergio Durigan Junior wrote:
> gdb/
> 2013-05-05  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	PR breakpoints/15413:
> 	* breakpoint.c (condition_completer): Rewrite parts of the code to
> 	handle completion of the "condition" command for pending
> 	breakpoints.
> 
> gdb/testsuite/
> 2013-05-05  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	PR breakpoints/15413:
> 	* gdb.base/pending.exp: Add test for completion of the "condition"
> 	command for pending breakpoints.
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 35ada7a..81bf5ed 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1013,25 +1013,36 @@ condition_completer (struct cmd_list_element *cmd,
>  
>        ALL_BREAKPOINTS (b)
>        {
> -	int single = b->loc->next == NULL;
> -	struct bp_location *loc;
> +	struct bp_location *loc = NULL;
>  	int count = 1;
>  
> -	for (loc = b->loc; loc; loc = loc->next)
> +	do
>  	  {
>  	    char location[50];
>  
> -	    if (single)
> -	      xsnprintf (location, sizeof (location), "%d", b->number);
> +	    if (b->loc == NULL)
> +	      {
> +		/* We're probably dealing with a pending breakpoint.  Just
> +		   inform its number.  */

s/probably//

s/inform/complete/ ?

> +		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.  :-)

> +	      }
>  
>  	    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'

-- 
Pedro Alves