Re: Patch: Little improvement to delete breakpoint command

Re: Patch: Little improvement to delete breakpoint command

[Markus Deuling]

Hi,

I renewed the patch and ran the testsuite with an without it:

without (gdb 6-5 vanilla)
# of expected passes            11033
# of unexpected failures        58

with the patch
# of expected passes            11037
# of unexpected failures        54



===============================================================

diff -urN gdb-6.5/gdb/breakpoint.c gdb-6.5_dev/gdb/breakpoint.c
--- gdb-6.5/gdb/breakpoint.c	2006-05-01 18:38:08.000000000 +0200
+++ gdb-6.5_dev/gdb/breakpoint.c	2006-11-29 13:44:43.000000000 +0100
@@ -6959,7 +6959,10 @@
  	    b->type != bp_thread_event &&
  	    b->type != bp_overlay_event &&
  	    b->number >= 0)
+        {
  	  breaks_to_delete = 1;
+	  break;
+        }
        }

        /* Ask user only if there are some breakpoints to delete.  */


Is that ok ?


Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: Patch: Little improvement to delete breakpoint command

[Daniel Jacobowitz]

On Wed, Nov 29, 2006 at 02:51:50PM +0100, Markus Deuling wrote:
> Hi,
> 
> I renewed the patch and ran the testsuite with an without it:

Thanks.  I checked this in, with this changelog entry:

2007-01-21  Markus Deuling  <deuling@de.ibm.com>

        * breakpoint.c (delete_command): Skip redundant loop iterations.


-- 
Daniel Jacobowitz
CodeSourcery

Patch: Little improvement to delete breakpoint command

[Markus Deuling]

Hello,

this little patch adds a break to the loop that is looking for 
breakpoints to delete in delete_command(). I think it is only necessary 
to find one breakpoint, there is no need to iterate further if found one.

===================================================================
--- breakpoint.old      2006-11-28 17:08:11.000000000 +0100
+++ breakpoint.c        2006-11-28 17:42:15.000000000 +0100
@@ -6960,6 +6960,7 @@
             b->type != bp_overlay_event &&
             b->number >= 0)
           breaks_to_delete = 1;
+         break;
        }

        /* Ask user only if there are some breakpoints to delete.  */



Regards,
Markus


-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: Patch: Little improvement to delete breakpoint command

[Daniel Jacobowitz]

On Tue, Nov 28, 2006 at 06:21:39PM +0100, Markus Deuling wrote:
> Hello,
> 
> this little patch adds a break to the loop that is looking for 
> breakpoints to delete in delete_command(). I think it is only necessary 
> to find one breakpoint, there is no need to iterate further if found one.
> 
> ===================================================================
> --- breakpoint.old      2006-11-28 17:08:11.000000000 +0100
> +++ breakpoint.c        2006-11-28 17:42:15.000000000 +0100
> @@ -6960,6 +6960,7 @@
>             b->type != bp_overlay_event &&
>             b->number >= 0)
>           breaks_to_delete = 1;
> +         break;
>        }
> 
>        /* Ask user only if there are some breakpoints to delete.  */
> 

Did you test this?  I'm pretty sure you're missing braces.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Patch: Little improvement to delete breakpoint command

[Markus Deuling]

Hello,

Daniel Jacobowitz schrieb:
> On Tue, Nov 28, 2006 at 06:21:39PM +0100, Markus Deuling wrote:
>> Hello,
>>
>> this little patch adds a break to the loop that is looking for 
>> breakpoints to delete in delete_command(). I think it is only necessary 
>> to find one breakpoint, there is no need to iterate further if found one.
>>
>> ===================================================================
>> --- breakpoint.old      2006-11-28 17:08:11.000000000 +0100
>> +++ breakpoint.c        2006-11-28 17:42:15.000000000 +0100
>> @@ -6960,6 +6960,7 @@
>>             b->type != bp_overlay_event &&
>>             b->number >= 0)
>>           breaks_to_delete = 1;
>> +         break;
>>        }
>>
>>        /* Ask user only if there are some breakpoints to delete.  */
>>
> 
> Did you test this?  I'm pretty sure you're missing braces.
> 

I did test it and it works for me.

The braces are included in vanilla gdb-6.5. Without that "break" the 
braces are unnecessary. This is the original routine:

       ALL_BREAKPOINTS (b)
       {
         if (b->type != bp_call_dummy &&
             b->type != bp_shlib_event &&
             b->type != bp_thread_event &&
             b->type != bp_overlay_event &&
             b->number >= 0)
           breaks_to_delete = 1;
       }


Regards,
Markus

Re: Patch: Little improvement to delete breakpoint command

[Daniel Jacobowitz]

On Tue, Nov 28, 2006 at 06:35:12PM +0100, Markus Deuling wrote:
> I did test it and it works for me.

Did you run the testsuite?  Or experiment with any case that should set
breaks_to_delete but not for the most recent breakpoint?

> The braces are included in vanilla gdb-6.5. Without that "break" the 
> braces are unnecessary. This is the original routine:
> 
>       ALL_BREAKPOINTS (b)
>       {
>         if (b->type != bp_call_dummy &&
>             b->type != bp_shlib_event &&
>             b->type != bp_thread_event &&
>             b->type != bp_overlay_event &&
>             b->number >= 0)
>           breaks_to_delete = 1;
>       }

Wrong braces.  You've converted that to:

       ALL_BREAKPOINTS (b)
       {
         if (b->type != bp_call_dummy &&
             b->type != bp_shlib_event &&
             b->type != bp_thread_event &&
             b->type != bp_overlay_event &&
             b->number >= 0)
           breaks_to_delete = 1;
           break;
       }

Now do you see the problem?  Remember, C is not whitespace sensitive.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Patch: Little improvement to delete breakpoint command

[Markus Deuling]

Hello,

Daniel Jacobowitz schrieb:
> On Tue, Nov 28, 2006 at 06:35:12PM +0100, Markus Deuling wrote:
>> I did test it and it works for me.
> 
> Did you run the testsuite?  Or experiment with any case that should set
> breaks_to_delete but not for the most recent breakpoint?
> 
>> The braces are included in vanilla gdb-6.5. Without that "break" the 
>> braces are unnecessary. This is the original routine:
>>
>>       ALL_BREAKPOINTS (b)
>>       {
>>         if (b->type != bp_call_dummy &&
>>             b->type != bp_shlib_event &&
>>             b->type != bp_thread_event &&
>>             b->type != bp_overlay_event &&
>>             b->number >= 0)
>>           breaks_to_delete = 1;
>>       }
> 
> Wrong braces.  You've converted that to:
> 
>        ALL_BREAKPOINTS (b)
>        {
>          if (b->type != bp_call_dummy &&
>              b->type != bp_shlib_event &&
>              b->type != bp_thread_event &&
>              b->type != bp_overlay_event &&
>              b->number >= 0)
>            breaks_to_delete = 1;
>            break;
>        }
> 
> Now do you see the problem?  Remember, C is not whitespace sensitive.
> 

You are right, sorry, silly mistake from me :-(
This should work:

--- breakpoint.old      2006-11-28 18:47:43.000000000 +0100
+++ breakpoint.c        2006-11-28 18:50:58.000000000 +0100
@@ -6959,7 +6959,10 @@
             b->type != bp_thread_event &&
             b->type != bp_overlay_event &&
             b->number >= 0)
-         breaks_to_delete = 1;
+         {
+           breaks_to_delete = 1;
+           break;
+         }
        }

        /* Ask user only if there are some breakpoints to delete.  */



Regards,
Markus

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com