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

Swen Schillig swen at vnet.ibm.com
Fri Mar 16 08:14:10 UTC 2018


On Fri, 2018-03-16 at 20:21 +1300, Andrew Bartlett via samba-technical
wrote:
> 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

Hmm, well nothing which couldn't be fixed by 
	sizeof(*call)
and
	talloc_set_name()

but I guess that would take all the good out of it and turn it into a
"I want to change something for the change sake" patch.

So I'm leaving it.

Thanks for reviewing !

Cheers Swen




More information about the samba-technical mailing list