On October 22, 2020 2:11:01 AM GMT+02:00, Stephen Hemminger [off-list ref] wrote:

On Thu, 22 Oct 2020 01:48:58 +0200
Petr Machata [off-list ref] wrote:

quoted

Stephen Hemminger [off-list ref] writes:

quoted

On Tue, 20 Oct 2020 22:43:37 +0200
Petr Machata [off-list ref] wrote:
 

quoted

Jakub Kicinski [off-list ref] writes:
 

quoted

On Tue, 20 Oct 2020 02:58:23 +0200 Petr Machata wrote:  

quoted

+static void dcb_ets_print_cbs(FILE *fp, const struct ieee_ets

*ets)

quoted

quoted

quoted

quoted

quoted

+{
+	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" :

"off");

quoted

quoted

quoted

quoted

quoted

+}  

I'd personally lean in the direction ethtool is taking and try

to limit

quoted

quoted

quoted

quoted

string values in json output as much as possible. This would be

a good

quoted

quoted

quoted

quoted

fit for bool.  

Yep, makes sense. The value is not user-toggleable, so the on /

off

quoted

quoted

quoted

there is just arbitrary.

I'll consider it for "willing" as well. That one is

user-toggleable, and

quoted

quoted

quoted

the "on" / "off" makes sense for consistency with the command

line. But

quoted

quoted

quoted

that doesn't mean it can't be a boolean in JSON.  

There are three ways of representing a boolean. You chose the

worst.

quoted

quoted

Option 1: is to use a json null value to indicate presence.
      this works well for a flag.
Option 2: is to use json bool.
	this looks awkward in non-json output
Option 3: is to use a string
     	but this makes the string output something harder to consume
	in json.  

What seems to be used commonly for these on/off toggles is the

following

quoted

pattern:

	print_string(PRINT_FP, NULL, "willing %s ", ets->willing ? "on" :

"off");

quoted

	print_bool(PRINT_JSON, "willing", NULL, true);

That way the JSON output is easy to query and the FP output is

obvious

quoted

and compatible with the command line. Does that work for you?

Yes, that is hybrid, maybe it should be a helper function?

Yep, I'll do the same dance as for the other helpers in the patch set. Currently this is cut and pasted all over the place.