Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
@ 2011-03-03  0:42 Michael Snyder
  2011-03-03  4:27 ` Joel Brobecker
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2011-03-03  0:42 UTC (permalink / raw)
  To: gdb-patches

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

This is what it does, and has done for >5 years, so I assume it is what 
it should do...

Agree?


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

2011-03-02  Michael Snyder  <msnyder@vmware.com>

	* breakpoint.c (create_breakpoint): 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	3 Mar 2011 00:38:01 -0000
@@ -7843,6 +7843,7 @@ create_breakpoint (struct gdbarch *gdbar
 	default:
 	  throw_exception (e);
 	}
+      /* FALLTHROUGH */
     default:
       if (!sals.nelts)
 	return 0;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
  2011-03-03  0:42 [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through Michael Snyder
@ 2011-03-03  4:27 ` Joel Brobecker
  2011-03-03  9:55   ` Pedro Alves
  2011-03-03 17:11   ` Michael Snyder
  0 siblings, 2 replies; 6+ messages in thread
From: Joel Brobecker @ 2011-03-03  4:27 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

> 2011-03-02  Michael Snyder  <msnyder@vmware.com>
> 
> 	* breakpoint.c (create_breakpoint): 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	3 Mar 2011 00:38:01 -0000
> @@ -7843,6 +7843,7 @@ create_breakpoint (struct gdbarch *gdbar
>  	default:
>  	  throw_exception (e);
>  	}
> +      /* FALLTHROUGH */
>      default:
>        if (!sals.nelts)
>  	return 0;

Just some thoughts:

It's actually never going to fall through, is it? Can we use "break;"
instead, even if we know it's never going to be reached? I think
it would make it clearer by not suggesting something that isn't
supposed to happen (the fall through).

-- 
Joel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
  2011-03-03  4:27 ` Joel Brobecker
@ 2011-03-03  9:55   ` Pedro Alves
  2011-03-03 17:11   ` Michael Snyder
  1 sibling, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2011-03-03  9:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Joel Brobecker, Michael Snyder

On Thursday 03 March 2011 04:26:52, Joel Brobecker wrote:
> > 2011-03-02  Michael Snyder  <msnyder@vmware.com>
> > 
> > 	* breakpoint.c (create_breakpoint): 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	3 Mar 2011 00:38:01 -0000
> > @@ -7843,6 +7843,7 @@ create_breakpoint (struct gdbarch *gdbar
> >  	default:
> >  	  throw_exception (e);
> >  	}
> > +      /* FALLTHROUGH */
> >      default:
> >        if (!sals.nelts)
> >  	return 0;
> 
> Just some thoughts:
> 
> It's actually never going to fall through, is it? Can we use "break;"
> instead, even if we know it's never going to be reached? I think
> it would make it clearer by not suggesting something that isn't
> supposed to happen (the fall through).

The root problem seems to be that lint hasn't been tought that
throw_exception doesn't return, so it thinks we do fall-through.
There are mechanisms to teach lint about the no-return property
of functions.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
  2011-03-03  4:27 ` Joel Brobecker
  2011-03-03  9:55   ` Pedro Alves
@ 2011-03-03 17:11   ` Michael Snyder
  2011-03-03 17:45     ` Tom Tromey
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2011-03-03 17:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

Joel Brobecker wrote:
>> 2011-03-02  Michael Snyder  <msnyder@vmware.com>
>>
>> 	* breakpoint.c (create_breakpoint): 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	3 Mar 2011 00:38:01 -0000
>> @@ -7843,6 +7843,7 @@ create_breakpoint (struct gdbarch *gdbar
>>  	default:
>>  	  throw_exception (e);
>>  	}
>> +      /* FALLTHROUGH */
>>      default:
>>        if (!sals.nelts)
>>  	return 0;
> 
> Just some thoughts:
> 
> It's actually never going to fall through, is it? Can we use "break;"
> instead, even if we know it's never going to be reached? I think
> it would make it clearer by not suggesting something that isn't
> supposed to happen (the fall through).
> 

No, honest, it *does* fall through!
I had to use a printf statement to convince myself.   ;-)

When the user answers "yes" to the query, it falls through.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
  2011-03-03 17:11   ` Michael Snyder
@ 2011-03-03 17:45     ` Tom Tromey
  2011-03-04 18:58       ` Michael Snyder
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2011-03-03 17:45 UTC (permalink / raw)
  To: Michael Snyder; +Cc: Joel Brobecker, gdb-patches

>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:

Michael> No, honest, it *does* fall through!
Michael> I had to use a printf statement to convince myself.   ;-)
Michael> When the user answers "yes" to the query, it falls through.

Yeah.  Your patch is correct, or at least correct-enough.
Adding a "break" would also be correct, because sals.nelts is never 0 at
the end of this case.

Tom


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through.
  2011-03-03 17:45     ` Tom Tromey
@ 2011-03-04 18:58       ` Michael Snyder
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Snyder @ 2011-03-04 18:58 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Joel Brobecker, gdb-patches

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

Tom Tromey wrote:
>>>>>> "Michael" == Michael Snyder <msnyder@vmware.com> writes:
> 
> Michael> No, honest, it *does* fall through!
> Michael> I had to use a printf statement to convince myself.   ;-)
> Michael> When the user answers "yes" to the query, it falls through.
> 
> Yeah.  Your patch is correct, or at least correct-enough.
> Adding a "break" would also be correct, because sals.nelts is never 0 at
> the end of this case.
> 
> Tom

OK then, I'd rather break than fall through.
Committed as attached.


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

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

	* breakpoint.c (create_breakpoint): Add missing break statement.

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:57:27 -0000
@@ -7843,6 +7843,7 @@ create_breakpoint (struct gdbarch *gdbar
 	default:
 	  throw_exception (e);
 	}
+      break;
     default:
       if (!sals.nelts)
 	return 0;

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-03-04 18:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-03  0:42 [RFA] breakpoint.c, create_breakpoint, document that the case statement falls through Michael Snyder
2011-03-03  4:27 ` Joel Brobecker
2011-03-03  9:55   ` Pedro Alves
2011-03-03 17:11   ` Michael Snyder
2011-03-03 17:45     ` Tom Tromey
2011-03-04 18:58       ` Michael Snyder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox