Thread (27 messages) 27 messages, 4 authors, 2020-11-05

Re: [PATCH iproute2-next v2 03/11] lib: utils: Add print_on_off_bool()

From: Petr Machata <hidden>
Date: 2020-11-02 23:05:32

Leon Romanovsky [off-list ref] writes:
On Sun, Nov 01, 2020 at 04:55:42PM -0700, David Ahern wrote:
quoted
On 10/31/20 3:23 PM, Petr Machata wrote:
quoted
David Ahern [off-list ref] writes:
quoted
On 10/30/20 6:29 AM, Petr Machata wrote:
quoted
diff --git a/lib/utils.c b/lib/utils.c
index 930877ae0f0d..8deec86ecbcd 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1763,3 +1763,11 @@ int parse_on_off(const char *msg, const char *realval, int *p_err)

 	return parse_one_of(msg, realval, values_on_off, ARRAY_SIZE(values_on_off), p_err);
 }
+
+void print_on_off_bool(FILE *fp, const char *flag, bool val)
+{
+	if (is_json_context())
+		print_bool(PRINT_JSON, flag, NULL, val);
+	else
+		fprintf(fp, "%s %s ", flag, val ? "on" : "off");
+}
I think print_on_off should be fine and aligns with parse_on_off once it
returns a bool.
print_on_off() is already used in the RDMA tool, and actually outputs
"on" and "off", unlike this. So I chose this instead.

I could rename the RDMA one though -- it's used in two places, whereas
this is used in about two dozen instances across the codebase.
yes, the rdma utils are using generic function names. The rdma version
should be renamed; perhaps rd_print_on_off. That seems to be once common
prefix. Added Leon.
I made fast experiment and the output for the code proposed here and existed
in the RDMAtool - result the same. So the good thing will be to delete the
function from the RDMA after print_on_off_bool() will be improved.
The RDMAtool uses literal "on" and "off" as values in JSON, not
booleans. Moving over to print_on_off_bool() would be a breaking change,
which is problematic especially in JSON output.
However I don't understand why print_on_off_bool() is implemented in
utils.c and not in lib/json_print.c that properly handles JSON context,
There's a whole lot of print_X functions for printing non-fundamental
data types in utils.c. Seemed obvious to put it there. I can move it to
json_print.c, no problem.

I think the current function does handle JSON context, what else do
you have in mind?
provide colorized output and doesn't require to supply FILE *fp.
Stephen Hemminger already pointed out the FILE *fp bit, I'll be removing
it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help