Re: [XEN PATCH v2 1/3] libxl: Fix nul-termination of the return value of libxl_xen_console_read_line()

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Aug 23, 2024 at 06:05:03PM +0100, Javi Merino wrote:
> diff --git a/tools/libs/light/libxl_console.c 
> b/tools/libs/light/libxl_console.c
> index a563c9d3c7f9..012fd996fba9 100644
> --- a/tools/libs/light/libxl_console.c
> +++ b/tools/libs/light/libxl_console.c
> @@ -774,12 +774,14 @@ libxl_xen_console_reader *
>  {
>      GC_INIT(ctx);
>      libxl_xen_console_reader *cr;
> -    unsigned int size = 16384;
> +    /* We want xen to fill the buffer in as few hypercalls as
> +     * possible, but xen will not nul-terminate it.  Leave one byte at
> +     * the end for the null */
> +    unsigned int size = 16384 + 1;

This comment doesn't really explain why the size choosen is 16k+1, it
kind of explain the +1 but that's about it.

16k seems to be the initial size
    
https://elixir.bootlin.com/xen/v4.19.0/source/xen/drivers/char/console.c#L110
But then, it is changed to depend on $(nproc) and loglevel
    
https://elixir.bootlin.com/xen/v4.19.0/source/xen/drivers/char/console.c#L1095
with 16k been the minimum it seems:
    
https://elixir.bootlin.com/xen/v4.19.0/source/xen/drivers/char/console.c#L1061

So, I think a useful comment here would reflect that 16k is default size
of Xen's console buffer.

Also, multi-line comments are normally expected to be with begin and end
markers on separated lines:
    /*
     * Comments.
     */


It be nice to fix both comments, but otherwise the patch looks good:
Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech