Re: [PATCH 6/7] gnttab: rename a parameter of gnttab_populate_status_frames()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/07/2026 2:52 pm, Jan Beulich wrote:
> In a function of this name, "nr" (and hence req_nr_frames) is, at the
> first and second glance, the number of frames to populate. That's
> misleading, and it's also only what one of the two callers
> (gnttab_grow_table()) actually passes in; the other (gnttab_set_version())
> passes the number of grant frames already in place.
>
> Fold declarations while at it, and convert the assignment to an
> initializer.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1807,12 +1807,9 @@ fault:
>  
>  static int
>  gnttab_populate_status_frames(struct domain *d, struct grant_table *gt,
> -                              unsigned int req_nr_frames)
> +                              unsigned int grant_frames)
>  {
> -    unsigned int i;
> -    unsigned int req_status_frames;
> -
> -    req_status_frames = grant_to_status_frames(req_nr_frames);
> +    unsigned int i, req_status_frames = grant_to_status_frames(grant_frames);
>  
>      /* Make sure, prior version checks are architectural visible */
>      block_speculation();
>

I fully agree that it's misnamed.

However, I'd suggest req_grant_frames to match the req_status_frames,
and also to hint at why it's not just gt->nr_grant_frames.

It might also be worth a comment saying /* The number of status frames
is always derived from the number of grant frames */ or something similar?

~Andrew