Re: [ndctl PATCH 2/3] ndctl/list.c: simplify flags handling

[Ross Zwisler <ross.zwisler@linux.intel.com>]

On Fri, Jun 29, 2018 at 04:57:29PM -0700, Dan Williams wrote:
> On Fri, Jun 29, 2018 at 4:39 PM, Ross Zwisler
> <ross.zwisler@linux.intel.com> wrote:
> > ndctl/list.c keeps track of a set of enum util_json_flags which are based
> > on the command line options passed in.  This handling is a little messy,
> > though.  We have an accessor function, listopts_to_flags(), which is used
> > multiple times.  We sometimes pass the flags around as function arguments,
> > and we also have a copy stashed in the local struct list_filter_arg called
> > "lfa".  In some functions we access the flags in multiple ways.
> >
> > These flags are local to ndctl/list.c, are set exactly once per invocation
> > and never change.  Create a variable local to that file, initialize it once
> > and use it everywhere.
> >
> > Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> > ---
> >  ndctl/list.c  | 54 +++++++++++++++++++++++++++---------------------------
> >  util/filter.h |  1 -
> >  2 files changed, 27 insertions(+), 28 deletions(-)
> 
> I don't see the point of this thrash. It's not a win code size wise
> and I think it needlessly adds more dependence on global variables.

I disagree.  We currently have the flags value passed around as function
arguments, we have a version stashed in a data structure we pass around, and
we have one which is already essentially a global variable that is accessed
via an accessor function.  Having all three is complex and unnecessary,
especially considering that the flags never change.

Can we just choose one way of accessing the flags and use it everywhere?  If
not a global variable, which would you like to use?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm