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