On Wed, Dec 26, 2018 at 4:03 PM Verma, Vishal L <[email protected]> wrote: > > On Sun, 2018-12-23 at 10:56 -0800, Dan Williams wrote: > > Functions that take a 'struct ndctl_ctx *' are fine to take the 'void *' > > argument representing a library context that is passed to all commands. > > However, the shared logging macros need a defined type to dereference > > and retrieve the log_ctx. Introduce _gen_ctx to generically represent > > 'struct ndctl_ctx *' and 'struct daxctl_ctx *' to the log helpers, and > > drop the remaining casts in ndctl/monitor.c. Add a couple BUILD_BUG_ON() > > statements to backup the log_ctx expectations. > > > > Cc: QI Fuli <[email protected]> > > Signed-off-by: Dan Williams <[email protected]> > > --- > > daxctl/lib/libdaxctl.c | 4 ++++ > > ndctl/lib/libndctl.c | 4 ++++ > > ndctl/monitor.c | 22 +++++++++++----------- > > util/log.h | 16 ++++++++++++---- > > 4 files changed, 31 insertions(+), 15 deletions(-) > > > > <..> > > > diff --git a/util/log.h b/util/log.h > > index 495e0d33c7f5..881971522690 100644 > > --- a/util/log.h > > +++ b/util/log.h > > @@ -56,10 +56,18 @@ do { \ > > # define log_notice(ctx, arg...) log_null(ctx, ## arg) > > #endif > > > > -#define dbg(x, arg...) log_dbg(&(x)->ctx, ## arg) > > -#define info(x, arg...) log_info(&(x)->ctx, ## arg) > > -#define err(x, arg...) log_err(&(x)->ctx, ## arg) > > -#define notice(x, arg...) log_notice(&(x)->ctx, ## arg) > > +struct _gen_ctx { > > + /* > > + * Requires ndctl_ctx and daxctl_ctx keep their log_ctx as their > > + * first element > > + */ > > + struct log_ctx ctx; > > +}; > > + > > +#define dbg(x, arg...) log_dbg(&((struct _gen_ctx *)(x))->ctx, ## arg) > > +#define info(x, arg...) log_info(&((struct _gen_ctx *)(x))->ctx, ## arg) > > +#define err(x, arg...) log_err(&((struct _gen_ctx *)(x))->ctx, ## arg) > > +#define notice(x, arg...) log_notice(&((struct _gen_ctx *)(x))->ctx, ## > > arg) > > Looks like this breaks ndctl/check.c's (ab)use of log_ctx directly, > causing a segfault when this is called.
Whoops. > I'm guessing this should be > fixed by passing the actual ndctl_ctx to the check function, and letting > the logging helpers dereference it correctly. > > It is also fixed if I move log_ctx to be the first member of btt_chk. > > Any thoughts on whether we should allow the embedding of log_ctx like > this? Yeah, I think we should, it's useful. I'll just cook up a patch to convert the command harness to be typesafe and drop this "offset-0" trickery. > Other than that the patch looks good to me. Good catch. _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
