qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Thomas Huth
Subject: Re: [PATCH] ui/console-vc: Silence warning about sprintf() on OpenBSD
Date: Tue, 15 Oct 2024 13:28:29 +0200
User-agent: Mozilla Thunderbird

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]