[libvirt] [PATCH 10/10] vbox: avoid sprintf

[Matthias Bolte]

2010/8/20 Eric Blake <eblake at redhat.com>:
> On 08/19/2010 04:14 PM, Matthias Bolte wrote:
>>>     if (guiPresent) {
>>>         if (guiDisplay) {
>>> -            sprintf(displayutf8, "DISPLAY=%.24s", guiDisplay);
>>> -            VBOX_UTF8_TO_UTF16(displayutf8, &env);
>>> +            char *displayutf8;
>>> +            if (virAsprintf(&displayutf8, "DISPLAY=%s", guiDisplay) < 0)
>>> +                virReportOOMError();
>>
>> Why don't we abort the try to start the guest when hitting OOM?
>> Starting the guest will probably fail because of OOM anyway.
>
> The existing code didn't worry about it, but you are probably right that
> we might as well give up harder on the first OOM.
>
>>
>> Also why switch to virAsprintf when snprintf would work too?
>
> Even this sprintf is currently safe (the %.24s fits into the length
> allocated for guiDisplay), but fragile and arbitrarily limited.  That
> is, the s[n]printf solution locks you into a particular length, and has
> to be revisited if someone ever has a reason why a 24-character display
> string is too short (and I can totally see someone complaining that
> their 25-byte FQDN is a reason to support a longer DISPLAY).  At least
> snprintf only has one place to touch (the array declaration length)
> instead of two with sprintf (the array declaration and the %.24s in the
> format string), but virAsprintf gets rid of the length limitation once
> and for all.

Ah. Okay I missed that guiDisplay is actually arbitrary input and
virAsprintf is better in that case.

ACK for using virAsprintf here.

Matthias