[RFA] breakpoint.c, enable_command, missing break statement.

[RFA] breakpoint.c, enable_command, missing break statement.

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

I'm not entirely sure what to do with this one.

It clearly falls through, and it works as is, so the least change
is just to comment it.


[-- Attachment #2: break9.txt --]
[-- Type: text/plain, Size: 639 bytes --]

2011-03-04  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* breakpoint.c (enable_command): Document that case falls through.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.546
diff -u -p -u -p -r1.546 breakpoint.c
--- breakpoint.c	1 Mar 2011 02:16:56 -0000	1.546
+++ breakpoint.c	4 Mar 2011 18:45:43 -0000
@@ -11042,6 +11042,7 @@ enable_command (char *args, int from_tty
       case bp_read_watchpoint:
       case bp_access_watchpoint:
 	enable_breakpoint (bpt);
+	/* FALLTHROUGH */
       default:
 	continue;
       }

Re: [RFA] breakpoint.c, enable_command, missing break statement.

[Pedro Alves]

On Friday 04 March 2011 18:49:21, Michael Snyder wrote:
> I'm not entirely sure what to do with this one.
> 
> It clearly falls through, and it works as is, so the least change
> is just to comment it.

It looks irrelevant.  The intent of the code is obviously
to iterate over all breakpoints, or all kinds.  
The "continue" is continuing the loop hidden in
ALL_BREAKPOINTS at the next iteration.  But if you replace
all the continue's with break's it will still work the same,
because the "break" would break the the "switch", not the
loop.  I think if you do that the code ends up simpler to
read, with no magic.

-- 
Pedro Alves

Re: [RFA] breakpoint.c, enable_command, missing break statement.

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

Pedro Alves wrote:
> On Friday 04 March 2011 18:49:21, Michael Snyder wrote:
>> I'm not entirely sure what to do with this one.
>>
>> It clearly falls through, and it works as is, so the least change
>> is just to comment it.
> 
> It looks irrelevant. 

It's always relevant to document a fall-through.

> The intent of the code is obviously
> to iterate over all breakpoints, or all kinds.  
> The "continue" is continuing the loop hidden in
> ALL_BREAKPOINTS at the next iteration.  But if you replace
> all the continue's with break's it will still work the same,
> because the "break" would break the the "switch", not the
> loop.  I think if you do that the code ends up simpler to
> read, with no magic.
> 

Agreed.  Extending the same fix to disable_command, and committing.


[-- Attachment #2: break9.txt --]
[-- Type: text/plain, Size: 1550 bytes --]

2011-03-04  Michael Snyder  <msnyder@msnyder-server.eng.vmware.com>

	* breakpoint.c (enable_command): Use break instead of continue,
	and fill in a missing break.
	(disable_command): Ditto.

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.546
diff -u -p -u -p -r1.546 breakpoint.c
--- breakpoint.c	1 Mar 2011 02:16:56 -0000	1.546
+++ breakpoint.c	4 Mar 2011 19:56:21 -0000
@@ -10929,7 +10929,7 @@ disable_command (char *args, int from_tt
       case bp_none:
 	warning (_("attempted to disable apparently deleted breakpoint #%d?"),
 		 bpt->number);
-	continue;
+	break;
       case bp_breakpoint:
       case bp_tracepoint:
       case bp_fast_tracepoint:
@@ -10941,8 +10941,9 @@ disable_command (char *args, int from_tt
       case bp_read_watchpoint:
       case bp_access_watchpoint:
 	disable_breakpoint (bpt);
+	break;
       default:
-	continue;
+	break;
       }
   else if (strchr (args, '.'))
     {
@@ -11030,7 +11031,7 @@ enable_command (char *args, int from_tty
       case bp_none:
 	warning (_("attempted to enable apparently deleted breakpoint #%d?"),
 		 bpt->number);
-	continue;
+	break;
       case bp_breakpoint:
       case bp_tracepoint:
       case bp_fast_tracepoint:
@@ -11042,8 +11043,9 @@ enable_command (char *args, int from_tty
       case bp_read_watchpoint:
       case bp_access_watchpoint:
 	enable_breakpoint (bpt);
+	break;
       default:
-	continue;
+	break;
       }
   else if (strchr (args, '.'))
     {

Re: [RFA] breakpoint.c, enable_command, missing break statement.

[Pedro Alves]

On Friday 04 March 2011 19:58:42, Michael Snyder wrote:
> > It looks irrelevant. 
> 
> It's always relevant to document a fall-through.

Sorry if I wasn't clear, bad choice of words, I guess.
For avoidance of doubt, what I was trying to say was
that it's the same thing whether we document that
this case fallsthrough or whether we a break.

> Agreed.  Extending the same fix to disable_command, and committing.

Thanks.

-- 
Pedro Alves