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

[Javi Merino <javi.merino@xxxxxxxxx>]

On Tue, Aug 27, 2024 at 03:20:10PM GMT, Anthony PERARD wrote:
> 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.

Ok, I'll add a line that explains this.

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

In this file, there were more comments that didn't have the end marker
in a separate line, so I was just trying to be consistent.  For
example:

- 
https://elixir.bootlin.com/xen/v4.19.0/source/tools/libs/light/libxl_console.c#L454
- 
https://elixir.bootlin.com/xen/v4.19.0/source/tools/libs/light/libxl_console.c#L752
  (this one is mixed actually)
- 
https://elixir.bootlin.com/xen/v4.19.0/source/tools/libs/light/libxl_console.c#L790

Sure, I will make all comments I introduce have an end marker on a separate line

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

Thanks for the review!
Javi