Re: [Qemu-block] [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries()

[Eric Blake]

On 05/11/2015 08:40 AM, Kevin Wolf wrote:

>>> +        char indexstr[slen], prefix[slen];
>>
>> And more dependence on a working C99 compiler, thanks to variable length
>> array (VLA).
>>
>>> +        size_t snprintf_ret;
>>> +
>>> +        snprintf_ret = snprintf(indexstr, slen, "%s%u", subqdict, i);
>>> +        assert(snprintf_ret < slen);
>>
>> Since gcc may compile the allocation of indexstr into a malloc()
>> anyways, would it be any simpler to just use g_strdup_printf() directly,
>> instead of futzing around with VLA and snprintf() ourselves?  It might
>> mean less code, as some of the error checking is taken care of on your
>> behalf.
> 
> This code parallels the code in qdict_array_split(), which looks almos
> the same, except that the latter doesn't support subqict and therefore
> has a fixed-size array rather than a VLA.
> 
> If you think that g_strdup_printf() is preferable, I would do that on
> top of this series and change both functions.

I'm not strongly opposed to keeping snprintf, but agree that if you want
to clean it up to g_strdup_printf(), a separate patch hitting multiple
uses would be cleaner than respinning this patch.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature