[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries()
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 01/34] qdict: Add qdict_array_entries() |
Date: |
Mon, 11 May 2015 09:28:52 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
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