[Top][All Lists]

[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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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