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

Andrew Cooper <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <[email protected]>
On 28/07/2026 7:50 am, [email protected] wrote:
> From: Denis Mukhin <[email protected]> 
>
> 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 <[email protected]>
> ---
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.