On 08.03.2024 12:51, Federico Serafini wrote:
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -130,9 +130,12 @@ static bool virq_is_global(unsigned int virq)
>
> case VIRQ_ARCH_0 ... VIRQ_ARCH_7:
> return arch_virq_is_global(virq);
> +
> + default:
> + ASSERT(virq < NR_VIRQS);
> + break;
> }
>
> - ASSERT(virq < NR_VIRQS);
> return true;
> }
Just for my understanding: The ASSERT() is moved so the "default" would
consist of more than just "break". Why is it that then the "return" isn't
moved, too?
> @@ -846,6 +849,7 @@ int evtchn_send(struct domain *ld, unsigned int lport)
> break;
> default:
> ret = -EINVAL;
> + break;
> }
I certainly agree here.
> @@ -1672,6 +1676,9 @@ static void domain_dump_evtchn_info(struct domain *d)
> case ECS_VIRQ:
> printk(" v=%d", chn->u.virq);
> break;
> + default:
> + /* Nothing to do in other cases. */
> + break;
> }
Yes this, just to mention it, while in line with what Misra demands is
pretty meaningless imo: The absence of "default" says exactly what the
comment now says. FTAOD - this is a comment towards the Misra guideline,
not so much towards the specific change here.
One other remark though, considering the specific function we're in: In
a certifiable environment, will there actually be the capability to
issue debug keys? Shouldn't we have a Kconfig option allowing to remove
all that from a build (and then also from relevant scans)?
Jan
