[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