Zhang Chen <[email protected]> writes:
> On Fri, Jun 26, 2026 at 4:46 PM Markus Armbruster <[email protected]> wrote:
>>
>> Looks like you missed my review of v8. I'm replaying my review
>> comments.
>
> Oh, my fault, very sorry about that, focused on other patch's comments fixes.
Happens :)
> Thank you for your detailed explanation.
>
> I will address your comments in next version.
> Expect the:
> IOThreadHolder *h = QAPI_CLONE(IOThreadHolder, holder);
>
> Same as your estimate, this doesn't seem worthwhile.
Replacing hand-written code by QAPI_CLONE() is worth a try.
I additionally described how to avoid the copy entirely instead, but
advised against it.
> And the same issue in the [ PATCH V8 14/15] comments:
> static IOThreadHolderList *iothread_get_holders_list(IOThread *iothread)
>
>> If iothread->holders was a IOThreadHolderList instead of a GList, this
>> loop could be QAPI_CLONE(IOThreadHolderList, iothread->holders) instead.
>
> If we change the GList *holders to the IOThreadHolderList *holders,
> This means that we have to reimplement the GList related API,
> for example the "g_list_prepend", "g_list_find_custom" and the
> "g_list_delete_link"
>
> This doesn't seem worthwhile.
The replacement for g_list_prepend() would be trivial:
h->next = iothread->holders;
iothread->holders = h;
iothread_unref() would turn into simple loop to find and delete the
matching list element. iothread_get_holders_list()'s similarly simple
loop around QAPI_CLONE() would turn into a single QAPI_CLONE().
Basically the same amount of code, I think.
Your solution already exists, which is a valid argument.
Mine stays within QAPI instead of mixing GList and QAPI. Also a valid
argument.
Use your judgement. I'd try to avoid the mixing.
