netdev - Re: [PATCH ethtool-next 2/5] pause: add --json support

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200924083627.613e5c59@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 24 Sep 2020 08:36:27 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     mkubecek@...e.cz, netdev@...r.kernel.org
Subject: Re: [PATCH ethtool-next 2/5] pause: add --json support

On Wed, 23 Sep 2020 17:10:30 -0700 Jacob Keller wrote:
> > -	printf("RX negotiated: %s\nTX negotiated: %s\n",
> > -	       rx_status ? "on" : "off", tx_status ? "on" : "off");
> > +
> > +	if (is_json_context()) {
> > +		open_json_object("negotiated");
> > +		print_bool(PRINT_JSON, "rx", NULL, rx_status);
> > +		print_bool(PRINT_JSON, "tx", NULL, tx_status);
> > +		close_json_object();
> > +	} else {
> > +		printf("RX negotiated: %s\nTX negotiated: %s\n",
> > +		       rx_status ? "on" : "off", tx_status ? "on" : "off");
> > +	}
> 
> Why not use print_string here like show_bool did?

Yeah.. I did not come up with a good way of reusing the show_bool code
so I gave up. Taking another swing at it - how does this look?

diff --git a/netlink/coalesce.c b/netlink/coalesce.c
index 65f75cf9a8dd..07a92d04b7a1 100644
--- a/netlink/coalesce.c
+++ b/netlink/coalesce.c
@@ -36,9 +36,9 @@ int coalesce_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	if (silent)
 		putchar('\n');
 	printf("Coalesce parameters for %s:\n", nlctx->devname);
-	printf("Adaptive RX: %s  TX: %s\n",
-	       u8_to_bool(tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_RX]),
-	       u8_to_bool(tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_TX]));
+	show_bool("rx", "Adaptive RX: %s  ",
+		  tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_RX]);
+	show_bool("tx", "TX: %s\n", tb[ETHTOOL_A_COALESCE_USE_ADAPTIVE_TX]);
 	show_u32(tb[ETHTOOL_A_COALESCE_STATS_BLOCK_USECS],
 		 "stats-block-usecs: ");
 	show_u32(tb[ETHTOOL_A_COALESCE_RATE_SAMPLE_INTERVAL],
diff --git a/netlink/netlink.h b/netlink/netlink.h
index 4916d25ed5c0..1012e8e32cd8 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -98,27 +98,30 @@ static inline void show_u32(const struct nlattr *attr, const char *label)
 		printf("%sn/a\n", label);
 }
 
-static inline const char *u8_to_bool(const struct nlattr *attr)
+static inline const char *u8_to_bool(const uint8_t *val)
 {
-	if (attr)
-		return mnl_attr_get_u8(attr) ? "on" : "off";
+	if (val)
+		return *val ? "on" : "off";
 	else
 		return "n/a";
 }
 
-static inline void show_bool(const char *key, const char *fmt,
-			     const struct nlattr *attr)
+static inline void show_bool_val(const char *key, const char *fmt, uint8_t *val)
 {
 	if (is_json_context()) {
-		if (attr) {
-			print_bool(PRINT_JSON, key, NULL,
-				   mnl_attr_get_u8(attr));
-		}
+		if (val)
+			print_bool(PRINT_JSON, key, NULL, val);
 	} else {
-		print_string(PRINT_FP, NULL, fmt, u8_to_bool(attr));
+		print_string(PRINT_FP, NULL, fmt, u8_to_bool(val));
 	}
 }
 
+static inline void show_bool(const char *key, const char *fmt,
+			     const struct nlattr *attr)
+{
+	show_bool_val(key, fmt, attr ? mnl_attr_get_payload(attr) : NULL);
+}
+
 /* misc */
 
 static inline void copy_devname(char *dst, const char *src)
diff --git a/netlink/pause.c b/netlink/pause.c
index f9dec9fe887a..5395398ba948 100644
--- a/netlink/pause.c
+++ b/netlink/pause.c
@@ -41,8 +41,8 @@ static int pause_autoneg_cb(const struct nlmsghdr *nlhdr, void *data)
 	struct pause_autoneg_status ours = {};
 	struct pause_autoneg_status peer = {};
 	struct nl_context *nlctx = data;
-	bool rx_status = false;
-	bool tx_status = false;
+	uint8_t rx_status = false;
+	uint8_t tx_status = false;
 	bool silent;
 	int err_ret;
 	int ret;
@@ -74,15 +74,10 @@ static int pause_autoneg_cb(const struct nlmsghdr *nlhdr, void *data)
 			tx_status = true;
 	}
 
-	if (is_json_context()) {
-		open_json_object("negotiated");
-		print_bool(PRINT_JSON, "rx", NULL, rx_status);
-		print_bool(PRINT_JSON, "tx", NULL, tx_status);
-		close_json_object();
-	} else {
-		printf("RX negotiated: %s\nTX negotiated: %s\n",
-		       rx_status ? "on" : "off", tx_status ? "on" : "off");
-	}
+	open_json_object("negotiated");
+	show_bool_val("rx", "RX negotiated: %s\n", &rx_status);
+	show_bool_val("tx", "TX negotiated: %s\n", &tx_status);
+	close_json_object();
 
 	return MNL_CB_OK;
 }