* [PATCH] xen/x86: Annotate deliberate fallthrough cases from XSA-154
@ 2016-02-18 12:26 Andrew Cooper
2016-02-18 13:38 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cooper @ 2016-02-18 12:26 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Coverity objects otherwise.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/mm.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index a05edc3..0bff7dd 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -924,10 +924,15 @@ get_page_from_l1e(
{
case 0:
break;
+
case 1:
if ( is_hardware_domain(l1e_owner) )
+ {
+ /* Fallthrough. */
case -1:
return 0;
+ }
+ /* Fallthrough. */
default:
ASSERT_UNREACHABLE();
}
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xen/x86: Annotate deliberate fallthrough cases from XSA-154
2016-02-18 12:26 [PATCH] xen/x86: Annotate deliberate fallthrough cases from XSA-154 Andrew Cooper
@ 2016-02-18 13:38 ` Jan Beulich
2016-02-18 14:05 ` Andrew Cooper
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2016-02-18 13:38 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 18.02.16 at 13:26, <andrew.cooper3@citrix.com> wrote:
> Coverity objects otherwise.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> ---
> xen/arch/x86/mm.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index a05edc3..0bff7dd 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -924,10 +924,15 @@ get_page_from_l1e(
> {
> case 0:
> break;
> +
> case 1:
> if ( is_hardware_domain(l1e_owner) )
> + {
> + /* Fallthrough. */
> case -1:
> return 0;
> + }
> + /* Fallthrough. */
> default:
This second fall-through is actually a bug (luckily noticable only
on debug builds).
I'll commit the patch suitably adjusted, albeit I have a hard time
seeing how
case 1:
if ( is_hardware_domain(l1e_owner) )
case -1:
cannot be seen as obviously deliberate. Or did Coverity perhaps
only complain about the second, indeed buggy one?
Jan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xen/x86: Annotate deliberate fallthrough cases from XSA-154
2016-02-18 13:38 ` Jan Beulich
@ 2016-02-18 14:05 ` Andrew Cooper
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2016-02-18 14:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 18/02/16 13:38, Jan Beulich wrote:
>>>> On 18.02.16 at 13:26, <andrew.cooper3@citrix.com> wrote:
>> Coverity objects otherwise.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> ---
>> xen/arch/x86/mm.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
>> index a05edc3..0bff7dd 100644
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -924,10 +924,15 @@ get_page_from_l1e(
>> {
>> case 0:
>> break;
>> +
>> case 1:
>> if ( is_hardware_domain(l1e_owner) )
>> + {
>> + /* Fallthrough. */
>> case -1:
>> return 0;
>> + }
>> + /* Fallthrough. */
>> default:
> This second fall-through is actually a bug (luckily noticable only
> on debug builds).
>
> I'll commit the patch suitably adjusted, albeit I have a hard time
> seeing how
>
> case 1:
> if ( is_hardware_domain(l1e_owner) )
> case -1:
>
> cannot be seen as obviously deliberate.
Many C programmers are not aware that it is even valid syntax.
The point of the MISSING_BREAK check is to second guess what the
programmer has actually written.
> Or did Coverity perhaps
> only complain about the second, indeed buggy one?
It only complained about the first. The second is a misaligned
unconditional return when considered in isolation.
~Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-18 14:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-18 12:26 [PATCH] xen/x86: Annotate deliberate fallthrough cases from XSA-154 Andrew Cooper
2016-02-18 13:38 ` Jan Beulich
2016-02-18 14:05 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.