[PATCH] ctdb: Replace mem-alloc/assign by move/memdup

Andrew Bartlett abartlet at samba.org
Fri Mar 16 07:21:07 UTC 2018


On Wed, 2018-03-14 at 16:10 +1100, Martin Schwenke via samba-technical
wrote:
> On Fri, 09 Mar 2018 09:05:25 +0100, Swen Schillig via samba-technical
> <samba-technical at lists.samba.org> wrote:
> 
> > Please review and push if OK.
> 
> Reviewed-by: Martin Schwenke <martin at meltin.net>

CTDB isn't my area, but I'm not sure if I like this.  The assignment
pattern is type safe (and really ends up dong the same thing), however
the memdup pattern is not type safe, because it takes and returns a
void *.  

In particular because sizeof(struct ctdb_call) is used, not
sizeof(*call), the type could be changed and then the memdup would be
wrong, and nobody would notice at compile time.

It also means the memory won't have the right type in the talloc name,
but the line number instead, which means you can't do talloc_get_type()
later. 

So I'm going to say 'no' sorry, because if it were 'my' code, I
wouldn't want this and while I know Martin has said yes, I think this
is a poor pattern. 

(A way out would be to extend talloc to provide a structure dup
function that uses typeof() or such tricks and so can't get it wrong). 

Sorry,

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba




More information about the samba-technical mailing list