[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