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

[Petr Machata <me@pmachata.org>]

Stephen Hemminger <stephen@networkplumber.org> 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);