Thread (24 messages) 24 messages, 4 authors, 2020-10-22

Re: [PATCH iproute2-next 15/15] dcb: Add a subtool for the DCB ETS object

From: Petr Machata <hidden>
Date: 2020-10-21 23:49:13

Stephen Hemminger [off-list ref] writes:
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)
+{
+	print_string(PRINT_ANY, "cbs", "cbs %s ", ets->cbs ? "on" : "off");
+}
I'd personally lean in the direction ethtool is taking and try to limit
string values in json output as much as possible. This would be a good
fit for bool.
Yep, makes sense. The value is not user-toggleable, so the on / off
there is just arbitrary.

I'll consider it for "willing" as well. That one is user-toggleable, and
the "on" / "off" makes sense for consistency with the command line. But
that doesn't mean it can't be a boolean in JSON.
There are three ways of representing a boolean. You chose the worst.
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
pattern:

	print_string(PRINT_FP, NULL, "willing %s ", ets->willing ? "on" : "off");
	print_bool(PRINT_JSON, "willing", NULL, true);

That way the JSON output is easy to query and the FP output is obvious
and compatible with the command line. Does that work for you?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help