Re: [PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt

[Ian Jackson <iwj@xxxxxxxxxxxxxx>]

Jan Beulich writes ("Re: [PATCH] tools/libxl: Remove page_size and page_shift 
from struct libxl_acpi_ctxt"):
> On 24.09.2021 13:05, Kevin Stefanov wrote:
> > As a result of recent work, two members of struct libxl_acpi_ctxt were
> > left with only one user. Thus, it becomes illogical for them to be
> > members of the struct at all.
> > 
> > Drop the two struct members and instead let the only function using
> > them have them as local variables.
> > 
> > Signed-off-by: Kevin Stefanov <kevin.stefanov@xxxxxxxxxx>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>

> I would like to suggest though to consider ...
> 
> > @@ -176,20 +174,19 @@ int libxl__dom_load_acpi(libxl__gc *gc,
> >          goto out;
> >      }
> >  
> > -    config.rsdp = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> > -    config.infop = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> > +    config.rsdp = (unsigned long)libxl__malloc(gc, page_size);
> > +    config.infop = (unsigned long)libxl__malloc(gc, page_size);
> >      /* Pages to hold ACPI tables */
> > -    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES *
> > -                                   libxl_ctxt.page_size);
> > +    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES * page_size);
> 
> ... using page_shift to replace all multiplications like the one here
> at this occasion.

I don't have an opinion about this; my tools ack can stand if this
change is made and reviewed.

Ian.