Re: [RFC PATCH 1/3] misra: address 5.5 pirq_cleanup_check

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/07/2025 10:24 pm, Dmytro Prokopchuk1 wrote:
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index c8c1bfa615..2efb5f5c78 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -672,7 +672,7 @@ static int evtchn_bind_pirq(evtchn_bind_pirq_t *bind)
>      if ( rc != 0 )
>      {
>          info->evtchn = 0;
> -        pirq_cleanup_check(info, d);
> +        PIRQ_CLEANUP_CHECK(info, d);

Well, this is awkward.  This is dead code, but only when you realise ...

> diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> index 95034c0d6b..958d0b1aca 100644
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -185,7 +185,7 @@ extern struct pirq *pirq_get_info(struct domain *d, int 
> pirq);
>  
>  void pirq_cleanup_check(struct pirq *pirq, struct domain *d);
>  
> -#define pirq_cleanup_check(pirq, d) \
> +#define PIRQ_CLEANUP_CHECK(pirq, d) \
>      (!(pirq)->evtchn ? pirq_cleanup_check(pirq, d) : (void)0)
>  

... what this is really doing.

Looking at this overall diff, it really is outrageous that we're hiding
a conditional call like this.

We should just remove the macro, and expand

    if ( !pirq->evtchn )
        pirq_cleanup_check(pirq, d);

in most of the callsites.  The overall diff will be smaller (no need to
change the comments), and the end result is proper regular normal C.

I can draft a patch to that effect.

~Andrew