[PATCH] Fix wrong talloc context, remove queue_destructor and minor updates.

[Amitay Isaacs]

Hi Swen,

On Tue, Dec 12, 2017 at 9:18 PM, Swen Schillig via samba-technical
<samba-technical at lists.samba.org> wrote:
> Hi Martin
> On Tue, 2017-12-12 at 15:37 +1100, Martin Schwenke wrote:
>> Hi Swen,
>>
> Thanks a lot for quick reply and the provided guidance.
>> Comments:
>>
>> * I don't think removing the talloc context argument is valid.  There
>>   is no guarantee that private_data points to a talloc context, so
> you're right,
> even though we might end up with a lot of other problems then.
> Anyway, I removed that change.
>>
>> * In the commit message for the destructor removal you could point to
>>   the commit that contains the removal of the only use of the
>>   "destroyed" structure member, which was really the only reason for
>>   the existence of the destructor.
> Hmm, if I understand it correctly, then it was wrong even back then
> when there was a consumer of "destroyed". Because right after calling
> the destructor that attribute would even have been free'd as well,
> right ?
>
> I've split it all up and hopefully explained my intention.
>
> I would really appreciate if you (and everybody else of course) could
> have a look again.

Patches look good, but I think they are incomplete.  The file
ctdb_io.c requires a bit of clean up.

I have abstracted the functionality of common/ctdb_io.c without
reference to ctdb_context in common/sock_io.c.  If you are interested
in improving the packet handling code, I would suggest you make the
relevant changes to common/sock_io.c. It already has some tests and
it's much easier to add tests for an abstraction that does not involve
ctdb_context.

The next step would be to simplify ctdb_io.c using sock_io
abstraction.  That way any improvements to sock_io are usable by ctdb
daemon and it avoids having to fix the code in two places.

Amitay.