2010/8/20 Eric Blake <eblake@xxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list