[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract li
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract list-valued options |
Date: |
Thu, 23 Mar 2017 14:18:03 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 03/23/2017 01:27 PM, Markus Armbruster wrote:
>>> -
>>> - num_entries = qdict_array_entries(options, prefix);
>>> + for (i = 0;; i++) {
>>> + sprintf(keybuf, "auth-supported.%d.auth", i);
>>
>> By my count, and including a trailing NUL, this is 21 bytes + the
>> maximum size of a formatted int to fit in keybuf[32]; 32-bit INT_MIN is
>> indeed 11 bytes. Cutting it close there, but I don't see an overflow
>> (if gcc 7's new -Wformat-truncation spots something, then gcc is too
>> strict.)
>
> 11 decimal digits take a hell of a list :)
>
> Could double the buffer if it makes anyone sleep better.
I'm okay with its current size.
>>> - value = host;
>>> - if (port) {
>>> - /* check for ipv6 */
>>> - if (strchr(host, ':')) {
>>> - strbuf = g_strdup_printf("[%s]:%s", host, port);
>>> - } else {
>>> - strbuf = g_strdup_printf("%s:%s", host, port);
>>
>> The old code only prints port information if it is present...
>>
>>> + host = qstring_get_str(qobject_to_qstring(val));
>>> + sprintf(keybuf, "server.%d.port", i);
>>> + port = qdict_get_str(options, keybuf);
>>>
>>> -
>>> - /* each iteration in the for loop will build upon the string, and
>>> if
>>> - * rados_str is NULL then it is our first pass */
>>> - if (rados_str) {
>>> - /* separate options with ';', as that is what rados_conf_set()
>>> - * requires */
>>> - rados_str_tmp = rados_str;
>>> - rados_str = g_strdup_printf("%s;%s", rados_str_tmp, value);
>>> - g_free(rados_str_tmp);
>>> + if (strchr(host, ':')) {
>>> + vals[i] = g_strdup_printf("[%s]:%s", host, port);
>>> } else {
>>> - rados_str = g_strdup(value);
>>> + vals[i] = g_strdup_printf("%s:%s", host, port);
>>
>> ...but the new code unconditionally prints port information, even when
>> port == NULL. Oops.
>
> How can port be null? SocketAddress member port is mandatory...
Indeed. Does that mean the old code had a dead branch? Looks like it!
So, if I'm not overlooking something, that means you've resolved all my
open questions, and can submit the patch as written with:
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH for-2.9 4/5] rbd: Peel off redundant RbdAuthMethod wrapper struct, (continued)
Re: [Qemu-devel] [PATCH for-2.9 3/5] rbd: Rewrite the code to extract list-valued options, Kevin Wolf, 2017/03/23