* [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