[ndctl PATCH 1/3] ndctl: simplify JSON print flag handling

[ndctl PATCH 1/3] ndctl: simplify JSON print flag handling

[Ross Zwisler]

json_object_to_json_string_ext()'s second argument is a flags field which
we always want to be JSON_C_TO_STRING_PRETTY.  We were going to a lot of
trouble for this, though.  We had multiple variables set to be this one
flag and util_display_json_array() took it as an argument, even though it
never varied.

Instead, just pass in the necessary flag when calling
json_object_to_json_string_ext(), removing the local variables and the extra
argument to util_display_json_array().

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 daxctl/list.c        |  5 ++---
 ndctl/bus.c          |  3 +--
 ndctl/dimm.c         |  3 +--
 ndctl/inject-smart.c |  3 +--
 ndctl/list.c         | 11 +++++------
 util/json.c          |  3 ++-
 util/json.h          |  2 +-
 7 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/daxctl/list.c b/daxctl/list.c
index 254f0ac..ed9e76c 100644
--- a/daxctl/list.c
+++ b/daxctl/list.c
@@ -50,7 +50,6 @@ static struct {
 };
 
 static int did_fail;
-static int jflag = JSON_C_TO_STRING_PRETTY;
 
 #define fail(fmt, ...) \
 do { \
@@ -130,9 +129,9 @@ int cmd_list(int argc, const char **argv, void *ctx)
 	}
 
 	if (jregions)
-		util_display_json_array(stdout, jregions, jflag);
+		util_display_json_array(stdout, jregions);
 	else if (jdevs)
-		util_display_json_array(stdout, jdevs, jflag);
+		util_display_json_array(stdout, jdevs);
 
 	if (did_fail)
 		return -ENOMEM;
diff --git a/ndctl/bus.c b/ndctl/bus.c
index fc31d06..b7b7d65 100644
--- a/ndctl/bus.c
+++ b/ndctl/bus.c
@@ -88,8 +88,7 @@ static int bus_action(int argc, const char **argv, const char *usage,
 	}
 
 	if (success)
-		util_display_json_array(stdout, jbuses,
-				JSON_C_TO_STRING_PRETTY);
+		util_display_json_array(stdout, jbuses);
 	else
 		json_object_put(jbuses);
 
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 1c16899..6964c5e 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -1081,8 +1081,7 @@ static int dimm_action(int argc, const char **argv, void *ctx,
 	}
 
 	if (actx.jdimms)
-		util_display_json_array(actx.f_out, actx.jdimms,
-				JSON_C_TO_STRING_PRETTY);
+		util_display_json_array(actx.f_out, actx.jdimms);
 
 	if (actx.f_out != stdout)
 		fclose(actx.f_out);
diff --git a/ndctl/inject-smart.c b/ndctl/inject-smart.c
index 60de9fe..2e62c89 100644
--- a/ndctl/inject-smart.c
+++ b/ndctl/inject-smart.c
@@ -375,8 +375,7 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
 		jhealth = util_dimm_health_to_json(dimm);
 		if (jhealth) {
 			json_object_object_add(jdimm, "health", jhealth);
-			util_display_json_array(stdout, jdimms,
-				JSON_C_TO_STRING_PRETTY);
+			util_display_json_array(stdout, jdimms);
 		}
 	}
 out:
diff --git a/ndctl/list.c b/ndctl/list.c
index 6cf7c39..030d73f 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -56,7 +56,6 @@ static unsigned long listopts_to_flags(void)
 struct util_filter_params param;
 
 static int did_fail;
-static int jflag = JSON_C_TO_STRING_PRETTY;
 
 #define fail(fmt, ...) \
 do { \
@@ -380,7 +379,7 @@ static int list_display(struct list_filter_arg *lfa)
 	struct json_object *jbuses = lfa->jbuses;
 
 	if (jbuses)
-		util_display_json_array(stdout, jbuses, jflag);
+		util_display_json_array(stdout, jbuses);
 	else if ((!!jdimms + !!jregions + !!jnamespaces) > 1) {
 		struct json_object *jplatform = json_object_new_object();
 
@@ -397,14 +396,14 @@ static int list_display(struct list_filter_arg *lfa)
 			json_object_object_add(jplatform, "namespaces",
 					jnamespaces);
 		printf("%s\n", json_object_to_json_string_ext(jplatform,
-					jflag));
+					JSON_C_TO_STRING_PRETTY));
 		json_object_put(jplatform);
 	} else if (jdimms)
-		util_display_json_array(stdout, jdimms, jflag);
+		util_display_json_array(stdout, jdimms);
 	else if (jregions)
-		util_display_json_array(stdout, jregions, jflag);
+		util_display_json_array(stdout, jregions);
 	else if (jnamespaces)
-		util_display_json_array(stdout, jnamespaces, jflag);
+		util_display_json_array(stdout, jnamespaces);
 	return 0;
 }
 
diff --git a/util/json.c b/util/json.c
index 94ed948..ff894c7 100644
--- a/util/json.c
+++ b/util/json.c
@@ -105,9 +105,10 @@ struct json_object *util_json_object_hex(unsigned long long val,
 	return jobj;
 }
 
-void util_display_json_array(FILE *f_out, struct json_object *jarray, int jflag)
+void util_display_json_array(FILE *f_out, struct json_object *jarray)
 {
 	int len = json_object_array_length(jarray);
+	int jflag = JSON_C_TO_STRING_PRETTY;
 
 	if (json_object_array_length(jarray) > 1)
 		fprintf(f_out, "%s\n", json_object_to_json_string_ext(jarray, jflag));
diff --git a/util/json.h b/util/json.h
index c5d1603..d0167cf 100644
--- a/util/json.h
+++ b/util/json.h
@@ -26,7 +26,7 @@ enum util_json_flags {
 };
 
 struct json_object;
-void util_display_json_array(FILE *f_out, struct json_object *jarray, int jflag);
+void util_display_json_array(FILE *f_out, struct json_object *jarray);
 struct json_object *util_bus_to_json(struct ndctl_bus *bus);
 struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		unsigned long flags);
-- 
2.14.4

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Ross Zwisler]

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(-)

diff --git a/ndctl/list.c b/ndctl/list.c
index 030d73f..32f679b 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -38,20 +38,7 @@ static struct {
 	bool firmware;
 } list;
 
-static unsigned long listopts_to_flags(void)
-{
-	unsigned long flags = 0;
-
-	if (list.idle)
-		flags |= UTIL_JSON_IDLE;
-	if (list.media_errors)
-		flags |= UTIL_JSON_MEDIA_ERRORS;
-	if (list.dax)
-		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
-	if (list.human)
-		flags |= UTIL_JSON_HUMAN;
-	return flags;
-}
+static unsigned long list_flags;
 
 struct util_filter_params param;
 
@@ -64,8 +51,7 @@ do { \
 			VERSION, __func__, __LINE__, ##__VA_ARGS__); \
 } while (0)
 
-static struct json_object *region_to_json(struct ndctl_region *region,
-		unsigned long flags)
+static struct json_object *region_to_json(struct ndctl_region *region)
 {
 	struct json_object *jregion = json_object_new_object();
 	struct json_object *jobj, *jbbs, *jmappings = NULL;
@@ -83,13 +69,13 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		goto err;
 	json_object_object_add(jregion, "dev", jobj);
 
-	jobj = util_json_object_size(ndctl_region_get_size(region), flags);
+	jobj = util_json_object_size(ndctl_region_get_size(region), list_flags);
 	if (!jobj)
 		goto err;
 	json_object_object_add(jregion, "size", jobj);
 
 	jobj = util_json_object_size(ndctl_region_get_available_size(region),
-			flags);
+			list_flags);
 	if (!jobj)
 		goto err;
 	json_object_object_add(jregion, "available_size", jobj);
@@ -118,7 +104,8 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 	iset = ndctl_region_get_interleave_set(region);
 	if (iset) {
 		jobj = util_json_object_hex(
-				ndctl_interleave_set_get_cookie(iset), flags);
+				ndctl_interleave_set_get_cookie(iset),
+				list_flags);
 		if (!jobj)
 			fail("\n");
 		else
@@ -147,7 +134,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 			json_object_object_add(jregion, "mappings", jmappings);
 		}
 
-		jmapping = util_mapping_to_json(mapping, listopts_to_flags());
+		jmapping = util_mapping_to_json(mapping, list_flags);
 		if (!jmapping) {
 			fail("\n");
 			continue;
@@ -162,7 +149,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		json_object_object_add(jregion, "state", jobj);
 	}
 
-	jbbs = util_region_badblocks_to_json(region, &bb_count, flags);
+	jbbs = util_region_badblocks_to_json(region, &bb_count, list_flags);
 	if (bb_count) {
 		jobj = json_object_new_int(bb_count);
 		if (!jobj) {
@@ -171,7 +158,7 @@ static struct json_object *region_to_json(struct ndctl_region *region,
 		}
 		json_object_object_add(jregion, "badblock_count", jobj);
 	}
-	if ((flags & UTIL_JSON_MEDIA_ERRORS) && jbbs)
+	if ((list_flags & UTIL_JSON_MEDIA_ERRORS) && jbbs)
 		json_object_object_add(jregion, "badblocks", jbbs);
 
 	pd = ndctl_region_get_persistence_domain(region);
@@ -222,7 +209,7 @@ static void filter_namespace(struct ndctl_namespace *ndns,
 					lfa->jnamespaces);
 	}
 
-	jndns = util_namespace_to_json(ndns, lfa->flags);
+	jndns = util_namespace_to_json(ndns, list_flags);
 	if (!jndns) {
 		fail("\n");
 		return;
@@ -256,7 +243,7 @@ static bool filter_region(struct ndctl_region *region,
 					lfa->jregions);
 	}
 
-	jregion = region_to_json(region, lfa->flags);
+	jregion = region_to_json(region);
 	if (!jregion) {
 		fail("\n");
 		return false;
@@ -297,7 +284,7 @@ static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
 			json_object_object_add(lfa->jbus, "dimms", lfa->jdimms);
 	}
 
-	jdimm = util_dimm_to_json(dimm, lfa->flags);
+	jdimm = util_dimm_to_json(dimm, list_flags);
 	if (!jdimm) {
 		fail("\n");
 		return;
@@ -323,7 +310,7 @@ static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
 	if (list.firmware) {
 		struct json_object *jfirmware;
 
-		jfirmware = util_dimm_firmware_to_json(dimm, lfa->flags);
+		jfirmware = util_dimm_firmware_to_json(dimm, list_flags);
 		if (jfirmware)
 			json_object_object_add(jdimm, "firmware", jfirmware);
 	}
@@ -412,6 +399,18 @@ static int num_list_flags(void)
 	return list.buses + list.dimms + list.regions + list.namespaces;
 }
 
+static void init_list_flags(void)
+{
+	if (list.idle)
+		list_flags |= UTIL_JSON_IDLE;
+	if (list.media_errors)
+		list_flags |= UTIL_JSON_MEDIA_ERRORS;
+	if (list.dax)
+		list_flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
+	if (list.human)
+		list_flags |= UTIL_JSON_HUMAN;
+}
+
 int cmd_list(int argc, const char **argv, void *ctx)
 {
 	const struct option options[] = {
@@ -470,12 +469,13 @@ int cmd_list(int argc, const char **argv, void *ctx)
 	if (num_list_flags() == 0)
 		list.namespaces = true;
 
+	init_list_flags();
+
 	fctx.filter_bus = filter_bus;
 	fctx.filter_dimm = list.dimms ? filter_dimm : NULL;
 	fctx.filter_region = filter_region;
 	fctx.filter_namespace = list.namespaces ? filter_namespace : NULL;
 	fctx.list = &lfa;
-	lfa.flags = listopts_to_flags();
 
 	rc = util_filter_walk(ctx, &fctx, &param);
 	if (rc)
diff --git a/util/filter.h b/util/filter.h
index effda24..c410df2 100644
--- a/util/filter.h
+++ b/util/filter.h
@@ -47,7 +47,6 @@ struct list_filter_arg {
 	struct json_object *jbuses;
 	struct json_object *jregion;
 	struct json_object *jbus;
-	unsigned long flags;
 };
 
 /*
-- 
2.14.4

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Dan Williams]

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.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Ross Zwisler]

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

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

[Dan Williams]

On Fri, Jun 29, 2018 at 5:29 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> 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?

The point of having them passed into the util_filter_param was in
preparation for the "ndctl monitor" work. I'd prefer to see how that
work re-uses the list helpers or what can be refactored before we
convert more code to just look at a global variable. I agree that it
is currently pointless, but we are close to adding another use of
util_filter_param, so I'd prefer to see where that goes first.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[ndctl PATCH 3/3] ndctl list: always output array without --human

[Ross Zwisler]

If "ndctl list" is only printing a single object it currently does not
print it in an array.  When printing multiple objects, though, an array is
used.  This simplifies the output for single objects when a human is
looking at it but it creates complications for parsers like jq.  jq
wants to know whether it is trying to parse an object or an array of
objects.

For example, here's how you print just the mode of a single namespace:

 # ndctl list -n namespace1.0 | jq ".mode"
 "raw"

This works on a single object.  If you have multiple objects in a JSON
array you do this:

  # ndctl list | jq -r '(.[]) | .mode'
  raw
  raw

If you want to write a generic script that works in both cases you have to
do something quite complicated like:

  # ndctl list | jq -r '((. | arrays | .[]), . | objects | .mode)'
  raw
  raw

Instead of pushing the burden on the user, have ndctl output consistently be
an array even if there is only one element.  This allows the parsing code
to be simpler and only worry about one case.

If the user specifies the --human or -u flags we will assume that a person
is looking at the output and not a script and will continue to output a
single element which is not part of an array.

Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
 daxctl/list.c        | 11 +++++++----
 ndctl/bus.c          |  2 +-
 ndctl/dimm.c         |  2 +-
 ndctl/inject-smart.c |  2 +-
 ndctl/list.c         |  8 ++++----
 util/json.c          |  5 +++--
 util/json.h          |  3 ++-
 7 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/daxctl/list.c b/daxctl/list.c
index ed9e76c..58c35fa 100644
--- a/daxctl/list.c
+++ b/daxctl/list.c
@@ -83,6 +83,7 @@ int cmd_list(int argc, const char **argv, void *ctx)
 	struct json_object *jregions = NULL;
 	struct json_object *jdevs = NULL;
 	struct daxctl_region *region;
+	unsigned long list_flags;
 	int i;
 
         argc = parse_options(argc, argv, options, u, 0);
@@ -100,6 +101,8 @@ int cmd_list(int argc, const char **argv, void *ctx)
 	if (num_list_flags() == 0)
 		list.devs = true;
 
+	list_flags = listopts_to_flags();
+
 	daxctl_region_foreach(ctx, region) {
 		struct json_object *jregion = NULL;
 
@@ -117,7 +120,7 @@ int cmd_list(int argc, const char **argv, void *ctx)
 			}
 
 			jregion = util_daxctl_region_to_json(region,
-					param.dev, listopts_to_flags());
+					param.dev, list_flags);
 			if (!jregion) {
 				fail("\n");
 				continue;
@@ -125,13 +128,13 @@ int cmd_list(int argc, const char **argv, void *ctx)
 			json_object_array_add(jregions, jregion);
 		} else if (list.devs)
 			jdevs = util_daxctl_devs_to_list(region, jdevs,
-					param.dev, listopts_to_flags());
+					param.dev, list_flags);
 	}
 
 	if (jregions)
-		util_display_json_array(stdout, jregions);
+		util_display_json_array(stdout, jregions, list_flags);
 	else if (jdevs)
-		util_display_json_array(stdout, jdevs);
+		util_display_json_array(stdout, jdevs, list_flags);
 
 	if (did_fail)
 		return -ENOMEM;
diff --git a/ndctl/bus.c b/ndctl/bus.c
index b7b7d65..bcb8c71 100644
--- a/ndctl/bus.c
+++ b/ndctl/bus.c
@@ -88,7 +88,7 @@ static int bus_action(int argc, const char **argv, const char *usage,
 	}
 
 	if (success)
-		util_display_json_array(stdout, jbuses);
+		util_display_json_array(stdout, jbuses, 0);
 	else
 		json_object_put(jbuses);
 
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 6964c5e..97643a3 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -1081,7 +1081,7 @@ static int dimm_action(int argc, const char **argv, void *ctx,
 	}
 
 	if (actx.jdimms)
-		util_display_json_array(actx.f_out, actx.jdimms);
+		util_display_json_array(actx.f_out, actx.jdimms, 0);
 
 	if (actx.f_out != stdout)
 		fclose(actx.f_out);
diff --git a/ndctl/inject-smart.c b/ndctl/inject-smart.c
index 2e62c89..8da893c 100644
--- a/ndctl/inject-smart.c
+++ b/ndctl/inject-smart.c
@@ -375,7 +375,7 @@ static int dimm_inject_smart(struct ndctl_dimm *dimm)
 		jhealth = util_dimm_health_to_json(dimm);
 		if (jhealth) {
 			json_object_object_add(jdimm, "health", jhealth);
-			util_display_json_array(stdout, jdimms);
+			util_display_json_array(stdout, jdimms, sctx.flags);
 		}
 	}
 out:
diff --git a/ndctl/list.c b/ndctl/list.c
index 32f679b..5d173d7 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -366,7 +366,7 @@ static int list_display(struct list_filter_arg *lfa)
 	struct json_object *jbuses = lfa->jbuses;
 
 	if (jbuses)
-		util_display_json_array(stdout, jbuses);
+		util_display_json_array(stdout, jbuses, list_flags);
 	else if ((!!jdimms + !!jregions + !!jnamespaces) > 1) {
 		struct json_object *jplatform = json_object_new_object();
 
@@ -386,11 +386,11 @@ static int list_display(struct list_filter_arg *lfa)
 					JSON_C_TO_STRING_PRETTY));
 		json_object_put(jplatform);
 	} else if (jdimms)
-		util_display_json_array(stdout, jdimms);
+		util_display_json_array(stdout, jdimms, list_flags);
 	else if (jregions)
-		util_display_json_array(stdout, jregions);
+		util_display_json_array(stdout, jregions, list_flags);
 	else if (jnamespaces)
-		util_display_json_array(stdout, jnamespaces);
+		util_display_json_array(stdout, jnamespaces, list_flags);
 	return 0;
 }
 
diff --git a/util/json.c b/util/json.c
index ff894c7..1332458 100644
--- a/util/json.c
+++ b/util/json.c
@@ -105,12 +105,13 @@ struct json_object *util_json_object_hex(unsigned long long val,
 	return jobj;
 }
 
-void util_display_json_array(FILE *f_out, struct json_object *jarray)
+void util_display_json_array(FILE *f_out, struct json_object *jarray,
+		unsigned long flags)
 {
 	int len = json_object_array_length(jarray);
 	int jflag = JSON_C_TO_STRING_PRETTY;
 
-	if (json_object_array_length(jarray) > 1)
+	if (json_object_array_length(jarray) > 1 || !(flags & UTIL_JSON_HUMAN))
 		fprintf(f_out, "%s\n", json_object_to_json_string_ext(jarray, jflag));
 	else if (len) {
 		struct json_object *jobj;
diff --git a/util/json.h b/util/json.h
index d0167cf..aa2e976 100644
--- a/util/json.h
+++ b/util/json.h
@@ -26,7 +26,8 @@ enum util_json_flags {
 };
 
 struct json_object;
-void util_display_json_array(FILE *f_out, struct json_object *jarray);
+void util_display_json_array(FILE *f_out, struct json_object *jarray,
+		unsigned long flags);
 struct json_object *util_bus_to_json(struct ndctl_bus *bus);
 struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		unsigned long flags);
-- 
2.14.4

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/3] ndctl: simplify JSON print flag handling

[Dan Williams]

On Fri, Jun 29, 2018 at 4:39 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> json_object_to_json_string_ext()'s second argument is a flags field which
> we always want to be JSON_C_TO_STRING_PRETTY.  We were going to a lot of
> trouble for this, though.  We had multiple variables set to be this one
> flag and util_display_json_array() took it as an argument, even though it
> never varied.
>
> Instead, just pass in the necessary flag when calling
> json_object_to_json_string_ext(), removing the local variables and the extra
> argument to util_display_json_array().

I was thinking of one day supporting a flag other than
JSON_C_TO_STRING_PRETTY and have it be controlled by the command line.
However that never materialized, and we could always use an
environment variable to change the json format flag.

So,

Acked-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [ndctl PATCH 1/3] ndctl: simplify JSON print flag handling

[Verma, Vishal L]

On Fri, 2018-06-29 at 16:51 -0700, Dan Williams wrote:
> On Fri, Jun 29, 2018 at 4:39 PM, Ross Zwisler
> <ross.zwisler@linux.intel.com> wrote:
> > json_object_to_json_string_ext()'s second argument is a flags field
> > which
> > we always want to be JSON_C_TO_STRING_PRETTY.  We were going to a
> > lot of
> > trouble for this, though.  We had multiple variables set to be this
> > one
> > flag and util_display_json_array() took it as an argument, even
> > though it
> > never varied.
> > 
> > Instead, just pass in the necessary flag when calling
> > json_object_to_json_string_ext(), removing the local variables and
> > the extra
> > argument to util_display_json_array().
> 
> I was thinking of one day supporting a flag other than
> JSON_C_TO_STRING_PRETTY and have it be controlled by the command
> line.
> However that never materialized, and we could always use an
> environment variable to change the json format flag.

Or use jq to do some reformatting. e.g. <cmd> | jq -c should be the the
same as using the JSON_C_TO_STRING_PLAIN flag.

> 
> So,
> 
> Acked-by: Dan Williams <dan.j.williams@intel.com>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm