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

[Thomas Huth <thuth@redhat.com>]

On 15/10/2024 10.14, Daniel P. Berrangé wrote:
> 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.

While I'm obviously old-schooled, too (since I used snprintf here in the 
first place), I agree with Daniel that the few cycles of improved 
performance likely aren't justified in this case here, so g_strdup_printf() 
is a better choice here indeed. I just sent a v2 with that change.

  Thomas