Re: [Xen-devel] [PATCH 12/10] libxl: New convenience macro CONTAINING_STRUCT

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-01-13 at 13:53 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH 12/10] libxl: New convenience 
> macro CONTAINING_STRUCT"):
> > On Mon, 2012-01-09 at 17:34 +0000, Ian Jackson wrote:
> > > Provide a convenient and type-safe wrapper which does the correct
> > > dance to subtract offsetof.
> ...
> > > + *    void GET_CONTAINING_STRUCT(outer_type *outer_var [NB:UPDATED],
> > > + *                               some_type *inner_ptr,
> > > + *                               member_name);
> > > + *    outer_type *CONTAINING_STRUCT(outer_type,
> > > + *                                  some_type *inner_ptr,
> > > + *                                  member_name);
> > > + * The semantics are that after:
> > > + *    outer_type outer, *outer_var;
> > > + *    member_type *inner_ptr = &outer->member_name;
> > > + *    GET_CONTAINING_STRUCT(outer_var, &outer_ptr->member_name, 
> > > member_name)
> > > + * The following hold:
> > > + *    CONTAINING_STRUCT(inner_ptr, outer_type, member_name) == outer_ptr
> > 
> > There is no outer_ptr in the givens, did you mean outer_var or something
> > else?
> 
> I meant &outer.  Fixed.
> 
> > It's not entirely clear to me what the distinction between the GET_ and
> > non GET_ variant is (just that one returns the thing and the other
> > updates a variable?) and/or why we would need both.
> 
> That's exactly the difference.
> 
> > The common operation is to go from inner_ptr to outer_ptr I think
> > and CONTAINING_STRUCT seems to fill that niche just fine.
> 
> The reason we need GET_CONTAINING_STRUCT is because we want to write
> this:
> 
>     libxl__ev_devstate *ds;
>     GET_CONTAINING_STRUCT(ds, watch, watch);
> 
> If we have to use CONTAINING_STRUCT, we have to write:
> 
>     libxl__ev_devstate *ds =
>         CONTAINING_STRUCT(libxl__ev_devstate, watch, watch);
> 
> which is really unnecessarily verbose, because of the repetition of
> libxl__ev_devstate.
> 
> > BTW, in Linux this is called container_of, which is maybe more familiar
> > to people?
> 
> I'm not attached to the name.  If we pick Linux's name we should make
> sure its semantics are identical, so the argument order should change.
> 
> Should it be called "container_of" or "CONTAINER_OF" ?
> 
> I still think we need something like the GET_... version, or some
> other construct that allows us to avoid writing out the type name
> twice.
> 
> Hmm, actually, in our version, you can write,
> 
>     libxl__ev_devstate *ds = CONTAINING_STRUCT(ds, watch, watch);
> 
> since we call typeof on the type argument.
> 
> So how about I change everything to use that pattern, rename it
> CONTAINER_OF, and remove the GET_ version ?

Sounds good.

RE "container_of" vs. "CONTAINER_OF" -- Linux uses lowercase but that is
against the usual style of macros being caps. I don't mind which.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel