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

Martin Schwenke martin at meltin.net
Fri Mar 16 09:03:14 UTC 2018


On Fri, 16 Mar 2018 20:21:07 +1300, Andrew Bartlett
<abartlet at samba.org> 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). 

Thanks, Andrew.  I missed the subtlety where the change loses
type-safety.  :-(

peace & happiness,
martin



More information about the samba-technical mailing list