Re: [PATCH] ui/console-vc: Silence warning about sprintf() on OpenBSD

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Oct 14, 2024 at 10:50:44PM +0300, Michael Tokarev wrote:
> On 14.10.2024 18:15, Daniel P. Berrangé wrote:
> 
> > These two lines are the only place in the code that uses the
> > 
> >     char response[40];
> > 
> > so even better than switching to snprintf, how about just taking
> > buffer size out of the picture:
> > 
> >    g_autofree *response =
> >        g_strdup_printf("\033[%d;%dR",
> >                        (s->y_base + s->y) % s->total_height + 1,
> >                        s->x + 1);
> >    vc_respond_str(vc, response);
> 
> What's the reason to perform memory allocation in trivial places
> like this?  If we're worrying about possible buffer size issue,
> maybe asprintf() is a better alternative for such small things?
> Fragmenting heap memory for no reason seems too much overkill.
> But I'm old-scool, so.. :)

This is not a performance sensitive path, and using g_strdup_printf
makes it robust against any futher changes in the future. In the
context of all the memory allocation QEMU does, I can't see this
making any difference to heap fragmentation whatsoever.

snprintf with fixed buffers should only be used where there's a
demonstratable performance win, and the return value actually
checked with an assert() to prove we're not overflowing.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|