[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 2/4] quorum: implement bdrv_add_child() and b
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [PATCH v5 2/4] quorum: implement bdrv_add_child() and bdrv_del_child() |
Date: |
Mon, 12 Oct 2015 13:56:21 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
On Fri 09 Oct 2015 05:51:55 PM CEST, Max Reitz <address@hidden> wrote:
>>>> + s->bs = g_renew(BlockDriverState *, s->bs, s->max_children + 1);
>>>> + s->bs[s->num_children] = NULL;
>>>> + s->max_children++;
>>>> + }
>>>
>>> Just a suggestion, please feel free to ignore it completely:
>>>
>>> You can drop the s->max_children field and just always call g_renew()
>>> with s->num_children + 1 as the @count parameter. There shouldn't be
>>> any (visible) performance penalty, but it would simplify the code.
>>
>> If s->num_children has decreased since the previous g_renew() call
>> (because the user called quorum_del_child()) that could actually reduce
>> the array size.
>
> Yes, it could. And that would be just fine. ;-)
>
> We'd just keep the array exactly as big as it needs to be. I find that
> pretty intuitive. It's just counter-intuitive if you think one should
> never use realloc() for reducing the size of a buffer (and I know I
> myself tend to write my code thinking that).
If the goal is to keep the array exactly as big as it needs to be then
we should use g_renew() in quorum_del_child()...
Anyway we're digressing :-) this array is one pointer per Quorum child,
so the amount of memory we're talking about here is probably negligible.
I'm fine with any solution.
Berto