[v2,iproute2,2/6] rdma: use standard flag for json

Commit Message

Stephen Hemminger Jan. 4, 2024, 1:13 a.m. UTC

The other iproute2 utils use variable json as flag.

Signed-off-by: Stephen Hemminger <[e-mail]>
---
 rdma/rdma.c  |  7 ++++---
 rdma/rdma.h  |  1 -
 rdma/res.c   |  5 ++---
 rdma/stat.c  |  4 ++--
 rdma/utils.c | 12 ++++++------
 5 files changed, 14 insertions(+), 15 deletions(-)

Comments

Petr Machata Jan. 4, 2024, 12:07 p.m. UTC | #1

Stephen Hemminger <[e-mail]> writes:

> The other iproute2 utils use variable json as flag.

Some do, some don't. dcb, devlink, rdma do not, the rest do.

Personally I'm not a fan of global state, and consider this patch to be
a step back in terms of best practices. I do realize this is how it's
done in iproute2. To the point that utils.h actually carries external
declarations for these variables. But to me those are inevitable warts,
not something to embrace and extend.

Objectively... whatever. There's only one instance of the context
structure anyway. It's just the overtones of a quick hack that irk me.

Anyway, the mechanical parts of the conversion look OK. But:

> diff --git a/rdma/stat.c b/rdma/stat.c
> index 28b1ad857219..6a3f8ca44892 100644
> --- a/rdma/stat.c
> +++ b/rdma/stat.c
> @@ -208,7 +208,7 @@ int res_get_hwcounters(struct rd *rd, struct nlattr *hwc_table, bool print)
>  
>  		nm = mnl_attr_get_str(hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_NAME]);
>  		v = mnl_attr_get_u64(hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_VALUE]);
> -		if (rd->pretty_output && !rd->json_output)
> +		if (rd->pretty_output)
>  			newline_indent(rd);

newline_indent() issues close_json_object(). Previously it wouldn't be
invoked in JSON mode, now it will be. Doesn't this break JSON output?
Same question for the hunk below.

>  		res_print_u64(rd, nm, v, hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_NAME]);
>  	}
> @@ -802,7 +802,7 @@ static int do_stat_mode_parse_cb(const struct nlmsghdr *nlh, void *data,
>  			} else {
>  				print_string(PRINT_FP, NULL, ",", NULL);
>  			}
> -			if (rd->pretty_output && !rd->json_output)
> +			if (rd->pretty_output)
>  				newline_indent(rd);
>  
>  			print_string(PRINT_ANY, NULL, "%s", name);

Patch

diff --git a/rdma/rdma.c b/rdma/rdma.c
index 8dc2d3e344be..60ba8c0e5594 100644
--- a/rdma/rdma.c
+++ b/rdma/rdma.c
@@ -8,6 +8,9 @@ 
 #include "version.h"
 #include "color.h"
 
+/* Global utils flags */
+int json;
+
 static void help(char *name)
 {
 	pr_out("Usage: %s [ OPTIONS ] OBJECT { COMMAND | help }\n"
@@ -96,7 +99,6 @@  int main(int argc, char **argv)
 	bool show_driver_details = false;
 	const char *batch_file = NULL;
 	bool show_details = false;
-	bool json_output = false;
 	bool show_raw = false;
 	bool force = false;
 	struct rd rd = {};
@@ -125,7 +127,7 @@  int main(int argc, char **argv)
 			show_raw = true;
 			break;
 		case 'j':
-			json_output = 1;
+			++json;
 			break;
 		case 'f':
 			force = true;
@@ -151,7 +153,6 @@  int main(int argc, char **argv)
 
 	rd.show_details = show_details;
 	rd.show_driver_details = show_driver_details;
-	rd.json_output = json_output;
 	rd.pretty_output = pretty;
 	rd.show_raw = show_raw;
 
diff --git a/rdma/rdma.h b/rdma/rdma.h
index 0bf77f4dcf9e..f6830c851fb1 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -68,7 +68,6 @@  struct rd {
 	struct nlmsghdr *nlh;
 	char *buff;
 	json_writer_t *jw;
-	int json_output;
 	int pretty_output;
 	bool suppress_errors;
 	struct list_head filter_list;
diff --git a/rdma/res.c b/rdma/res.c
index 715cf93c4fab..f64224e1f3eb 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -160,7 +160,7 @@  void print_comm(struct rd *rd, const char *str, struct nlattr **nla_line)
 	if (!str)
 		return;
 
-	if (nla_line[RDMA_NLDEV_ATTR_RES_PID] || rd->json_output)
+	if (nla_line[RDMA_NLDEV_ATTR_RES_PID] || is_json_context())
 		snprintf(tmp, sizeof(tmp), "%s", str);
 	else
 		snprintf(tmp, sizeof(tmp), "[%s]", str);
@@ -187,8 +187,7 @@  void print_link(struct rd *rd, uint32_t idx, const char *name, uint32_t port,
 		snprintf(tmp, sizeof(tmp), "%s/-", name);
 	}
 
-	if (!rd->json_output)
-		print_string(PRINT_ANY, NULL, "link %s ", tmp);
+	print_string(PRINT_FP, NULL, "link %s ", tmp);
 }
 
 void print_qp_type(struct rd *rd, uint32_t val)
diff --git a/rdma/stat.c b/rdma/stat.c
index 28b1ad857219..6a3f8ca44892 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -208,7 +208,7 @@  int res_get_hwcounters(struct rd *rd, struct nlattr *hwc_table, bool print)
 
 		nm = mnl_attr_get_str(hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_NAME]);
 		v = mnl_attr_get_u64(hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_VALUE]);
-		if (rd->pretty_output && !rd->json_output)
+		if (rd->pretty_output)
 			newline_indent(rd);
 		res_print_u64(rd, nm, v, hw_line[RDMA_NLDEV_ATTR_STAT_HWCOUNTER_ENTRY_NAME]);
 	}
@@ -802,7 +802,7 @@  static int do_stat_mode_parse_cb(const struct nlmsghdr *nlh, void *data,
 			} else {
 				print_string(PRINT_FP, NULL, ",", NULL);
 			}
-			if (rd->pretty_output && !rd->json_output)
+			if (rd->pretty_output)
 				newline_indent(rd);
 
 			print_string(PRINT_ANY, NULL, "%s", name);
diff --git a/rdma/utils.c b/rdma/utils.c
index f73a9f19b617..32e12a64193a 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -572,7 +572,7 @@  int rd_exec_link(struct rd *rd, int (*cb)(struct rd *rd), bool strict_port)
 	uint32_t port;
 	int ret = 0;
 
-	new_json_obj(rd->json_output);
+	new_json_obj(json);
 	if (rd_no_arg(rd)) {
 		list_for_each_entry(dev_map, &rd->dev_map_list, list) {
 			rd->dev_idx = dev_map->idx;
@@ -621,7 +621,7 @@  int rd_exec_dev(struct rd *rd, int (*cb)(struct rd *rd))
 	struct dev_map *dev_map;
 	int ret = 0;
 
-	new_json_obj(rd->json_output);
+	new_json_obj(json);
 	if (rd_no_arg(rd)) {
 		list_for_each_entry(dev_map, &rd->dev_map_list, list) {
 			rd->dev_idx = dev_map->idx;
@@ -794,7 +794,7 @@  static int print_driver_string(struct rd *rd, const char *key_str,
 static int print_driver_s32(struct rd *rd, const char *key_str, int32_t val,
 			      enum rdma_nldev_print_type print_type)
 {
-	if (!rd->json_output) {
+	if (!is_json_context()) {
 		switch (print_type) {
 		case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
 			return pr_out("%s %d ", key_str, val);
@@ -811,7 +811,7 @@  static int print_driver_s32(struct rd *rd, const char *key_str, int32_t val,
 static int print_driver_u32(struct rd *rd, const char *key_str, uint32_t val,
 			      enum rdma_nldev_print_type print_type)
 {
-	if (!rd->json_output) {
+	if (!is_json_context()) {
 		switch (print_type) {
 		case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
 			return pr_out("%s %u ", key_str, val);
@@ -828,7 +828,7 @@  static int print_driver_u32(struct rd *rd, const char *key_str, uint32_t val,
 static int print_driver_s64(struct rd *rd, const char *key_str, int64_t val,
 			      enum rdma_nldev_print_type print_type)
 {
-	if (!rd->json_output) {
+	if (!is_json_context()) {
 		switch (print_type) {
 		case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
 			return pr_out("%s %" PRId64 " ", key_str, val);
@@ -845,7 +845,7 @@  static int print_driver_s64(struct rd *rd, const char *key_str, int64_t val,
 static int print_driver_u64(struct rd *rd, const char *key_str, uint64_t val,
 			      enum rdma_nldev_print_type print_type)
 {
-	if (!rd->json_output) {
+	if (!is_json_context()) {
 		switch (print_type) {
 		case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
 			return pr_out("%s %" PRIu64 " ", key_str, val);