Re: [PATCH v3 1/2] xen/arm: exclude xen,reg from direct-map domU extended regions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.05.2025 21:54, Stewart Hildebrand wrote:
> --- a/xen/common/rangeset.c
> +++ b/xen/common/rangeset.c
> @@ -433,6 +433,20 @@ bool rangeset_is_empty(
>      return ((r == NULL) || list_empty(&r->range_list));
>  }
>  
> +int rangeset_count_ranges(const struct rangeset *r)
> +{
> +    int nr = 0;

Ehem - this and the function's return type want to be unsigned.

> +    struct list_head *list;
> +
> +    if ( r == NULL )
> +        return 0;
> +
> +    list_for_each( list, &r->range_list )

Nit: Either you deem list_for_each a pseudo-keyword (then a blank is
missing) or you don't (then there are excess blanks).

Further I don't think this is valid to do without holding the rangeset's
lock in read mode (irrespective of the function return value potentially
being stale by the time the caller gets to look at it, which is no
different from other functions, i.e. falls in the caller's
responsibilities).

> +        nr++;

And then, if already abstraction is wanted, wouldn't this loop better be
yet another helper (macro?) in xen/list.h?

> +    return nr;
> +}

Finally: If this is to be commonly used in several places, having such a
helper is likely fine. As it stands, the sole caller is an __init
function, and hence this is unreachable code post-init (which while not
formally a Misra violation in my eyes effectively still is one). Aiui
the same can be achieved using rangeset_report_ranges(), with a new
(__init and static) callback function.

Jan