Re: [Xen-devel] [PATCH v2 1/4] xen/console: Don't treat NUL character as the end of the buffer

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 08/08/2019 14:51, Jan Beulich wrote:
> On 05.08.2019 15:29, Julien Grall wrote:
> > @@ -1261,14 +1259,15 @@ void debugtrace_printk(const char *fmt, ...)
> >       ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
> >    
> >       va_start(args, fmt);
> > -    vsnprintf(buf, sizeof(buf), fmt, args);
> > +    nr = vscnprintf(buf, sizeof(buf), fmt, args);
> >       va_end(args);
> >   
> >       if ( debugtrace_send_to_console )
> >       {
> > -        snprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
> > -        serial_puts(sercon_handle, cntbuf);
> > -        serial_puts(sercon_handle, buf);
> > +        unsigned int n = snprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
> While - given the size of cntbuf - the difference is mostly
> benign, you using vscnprintf() above calls for you also
> using scnprintf() here.
Good point, it would be safer too. I will update it.

> > --- a/xen/include/xen/video.h
> > +++ b/xen/include/xen/video.h
> > @@ -13,11 +13,11 @@
> >   
> >   #ifdef CONFIG_VIDEO
> >   void video_init(void);
> > -extern void (*video_puts)(const char *);
> > +extern void (*video_puts)(const char *, size_t nr);
> >   void video_endboot(void);
> >   #else
> >   #define video_init()    ((void)0)
> > -#define video_puts(s)   ((void)0)
> > +#define video_puts(s, nr)   ((void)0)
> While I don't think there's overly much risk of "s" getting an
> argument with side effects passed, I think that for "nr" the
> risk is there. May I ask that you evaluate both here, just in
> case?
Are you happy with the following code (Not yet compiled!):

#define video_ptus(s, nr) ((void)(s), (void)(nr))

> Preferably with these adjustments
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Thank you!

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel