On Wed, Feb 28, 2018 at 10:38:35AM -0600, Steve Wise wrote:
>
> > > > > > Steve,
> > > > > >
> > > > > > I imagined such function callback code a little bit simpler than
> it is
> > > > > now:
> > > > > >
> > > > > > nldev_res_get_XXX_dumpit {
> > > > > > initialization
> > > > > > checks
> > > > > >
> > > > > > ->callback(skb, RDMA_RESTRACK_XXX)
> > > > > >
> > > > > > close/error handling
> > > > > > }
> > > > > >
> > > > > > It will simplify the callback table a bit.
> > > > >
> > > > > That would work, at the expense of duplicating the parsing of the nl
> req
> > and
> > > > > building of the nl reply table. I'll look into it. It might make
> things a
> > > > > little cleaner.
> > > >
> > > > They can be static internal helpers, so no actual duplication will be.
> > >
> > > And after more thinking, it seems that you don't need callback too,
> > > simple wrapper function will be enough (everything is declared in one
> > > file, no need to dynamically change function callback, export it to
> > > other modules e.t.c).
> > >
> >
> > So you propose, I think:
> >
> > remove the logic in res_get_common_dumpit() that requires passing it the
> > nldev_cmd and nldev_attr values. Move these to helper functions and call
> them
> > directly from the nldev_res_get_XXX_dumpit() functions. This allows
> removing
> > the nldev_fill_res_entry struct entirely.
> >
> > But we still need to either pass the resource-specific fill function
> pointer to
> > res_get_common_dumpit(), or have a helper function that switches on
> res_type
> > and calls the appropriate fill function. Are you saying you propose the
> latter vs
> > passing the function ptr?
> >
>
> I'm inclined to leave the design as I have done it. I'm not sure having
> each nldev_res_get_XXX_dumpit() function do something like below makes it
> easier to read or more maintainable:
I like this version with the fill callback, makes a lot of sense to
me. Having only fill vary per type seems good with the way things are
now.
But not sure what picture Leon has to compare it with..
BTW:
+static struct nldev_fill_res_entry fill_entries[RDMA_RESTRACK_MAX] = {
+ [RDMA_RESTRACK_QP] = {
Missing 'const'
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
