Re: [Xen-devel] [PATCH] xen/evtchn: Cleanup for virq_is_global() infrastructure

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.01.18 at 17:16, <andrew.cooper3@xxxxxxxxxx> wrote:
> Switch it, and the arch infrastructure, to return bool.  Drop the unnecessary
> rc parameter, and remove a redundant assertion from send_global_virq().

s/parameter/variable/?

> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with two further remarks:

> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -93,29 +93,23 @@ static uint8_t 
> get_xen_consumer(xen_event_channel_notification_t fn)
>  /* Get the notification function for a given Xen-bound event channel. */
>  #define xen_notification_fn(e) (xen_consumers[(e)->xen_consumer-1])
>  
> -static int virq_is_global(uint32_t virq)
> +static bool virq_is_global(uint32_t virq)

Also make the parameter unsigned int at this occasion?

>  {
> -    int rc;
> -
> -    ASSERT(virq < NR_VIRQS);
> -
>      switch ( virq )
>      {
>      case VIRQ_TIMER:
>      case VIRQ_DEBUG:
>      case VIRQ_XENOPROF:
>      case VIRQ_XENPMU:
> -        rc = 0;
> -        break;
> +        return false;
> +
>      case VIRQ_ARCH_0 ... VIRQ_ARCH_7:
> -        rc = arch_virq_is_global(virq);
> -        break;
> +        return arch_virq_is_global(virq);
> +
>      default:
> -        rc = 1;
> -        break;
> +        ASSERT(virq < NR_VIRQS);
> +        return true;
>      }
> -
> -    return rc;
>  }

Could I talk you into dropping the "default:" and instead put
ASSERT() and return outside of the switch()? I generally think that
where this is possible without causing compiler warnings, it results
in a more "normal" look of the overall function (in particular with a
return at the function's end, rather than giving the appearance -
without looking at all the case blocks inside the switch - that the
function returns void).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel