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 :|
