Re: [PATCH v8 5/7] xen/console: use memcpy() in conring_puts()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 28/07/2026 7:50 am, dmukhin@xxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx> 
>
> Make conring_puts() more efficient by using memcpy()'s, rather than
> copying the ring a byte at a time.
>
> No functional change intended.
>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Changes since v7:
> - hardended len check in conring_puts()
> ---
>  xen/drivers/char/console.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 09282a7a4f8e..a1b8e5f5b507 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -361,12 +361,24 @@ static DECLARE_SOFTIRQ_TASKLET(conring_tasklet, 
> conring_notify, NULL);
>  /* NB: Do not send conring VIRQs during panic. */
>  static bool conring_no_notify;
>  
> -static void conring_puts(const char *str, size_t len)
> +static void conring_puts(const char *str, unsigned int len)

size_t is the only correct type to be using here.  Anything else is
buggy and a ...

>  {
> +    unsigned int src = len;
> +
> +    /* There are no callers with strings longer than PAGE_SIZE. */
> +    BUG_ON(len > PAGE_SIZE);

... bug waiting to happen.  Switching to ASSERT() ok either; it is fine
to pass more than a page here, and all this does is screw over some
future person who has a complicated debugging scenario.

I have 0 remaining patients for the avoidance of size_t.  I will nack
any further patches I see doing it, as well as any further advise I see
given from anyone in the community.


The rest of the patch is fine.  Please resubmit while keeping len as size_t.

~Andrew