lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <874jft6v4b.fsf@nvidia.com>
Date: Thu, 04 Jan 2024 13:07:38 +0100
From: Petr Machata <me@...chata.org>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: leon@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2 iproute2 2/6] rdma: use standard flag for json


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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ