[PATCH ethtool 0/7] compiler warnings cleanup, part 2

[PATCH ethtool 0/7] compiler warnings cleanup, part 2

[Michal Kubecek]

Two compiler warnings still appear when compiling current source:
comparison between signed and unsigned values and missing struct member
initializations.

The former are mostly handled by declaring variables (loop iterators,
mostly) as unsigned, only few required an explicit cast. The latter are
handled by using named field initializers; in link_mode_info[] array,
helper macros are also used to make code easier to read and check.

As the final step, add "-Wextra" to default CFLAGS to catch any future
warnings as early as possible.

Michal Kubecek (7):
  netlink: get rid of signed/unsigned comparison warnings
  ioctl: check presence of eeprom length argument properly
  ioctl: get rid of signed/unsigned comparison warnings
  get rid of signed/unsigned comparison warnings in register dump
    parsers
  settings: simplify link_mode_info[] initializers
  ioctl: convert cmdline_info arrays to named initializers
  build: add -Wextra to default CFLAGS

 Makefile.am        |   2 +-
 dsa.c              |   2 +-
 ethtool.c          | 435 ++++++++++++++++++++++++++++++++++-----------
 fec.c              |   2 +-
 ibm_emac.c         |   2 +-
 marvell.c          |   2 +-
 natsemi.c          |   2 +-
 netlink/features.c |   4 +-
 netlink/netlink.c  |   4 +-
 netlink/netlink.h  |   2 +-
 netlink/nlsock.c   |   2 +-
 netlink/parser.c   |   2 +-
 netlink/settings.c | 242 ++++++++++---------------
 rxclass.c          |   8 +-
 sfpdiag.c          |   2 +-
 tg3.c              |   4 +-
 16 files changed, 439 insertions(+), 278 deletions(-)

-- 
2.28.0

[PATCH ethtool 1/7] netlink: get rid of signed/unsigned comparison warnings

[Michal Kubecek]

Get rid of compiler warnings about comparison between signed and
unsigned integer values in netlink code.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 netlink/features.c | 4 ++--
 netlink/netlink.c  | 4 ++--
 netlink/netlink.h  | 2 +-
 netlink/nlsock.c   | 2 +-
 netlink/parser.c   | 2 +-
 netlink/settings.c | 6 +++---
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/netlink/features.c b/netlink/features.c
index 8b5b8588ca23..f5862e97a265 100644
--- a/netlink/features.c
+++ b/netlink/features.c
@@ -149,7 +149,7 @@ int dump_features(const struct nlattr *const *tb,
 			continue;
 
 		for (j = 0; j < results.count; j++) {
-			if (feature_flags[j] == i) {
+			if (feature_flags[j] == (int)i) {
 				n_match++;
 				flag_value = flag_value ||
 					feature_on(results.active, j);
@@ -163,7 +163,7 @@ int dump_features(const struct nlattr *const *tb,
 		for (j = 0; j < results.count; j++) {
 			const char *name = get_string(feature_names, j);
 
-			if (feature_flags[j] != i)
+			if (feature_flags[j] != (int)i)
 				continue;
 			if (n_match == 1)
 				dump_feature(&results, NULL, NULL, j,
diff --git a/netlink/netlink.c b/netlink/netlink.c
index 76b6e825b1d0..e42d57076a4b 100644
--- a/netlink/netlink.c
+++ b/netlink/netlink.c
@@ -33,9 +33,9 @@ int nomsg_reply_cb(const struct nlmsghdr *nlhdr, void *data __maybe_unused)
 int attr_cb(const struct nlattr *attr, void *data)
 {
 	const struct attr_tb_info *tb_info = data;
-	int type = mnl_attr_get_type(attr);
+	uint16_t type = mnl_attr_get_type(attr);
 
-	if (type >= 0 && type <= tb_info->max_type)
+	if (type <= tb_info->max_type)
 		tb_info->tb[type] = attr;
 
 	return MNL_CB_OK;
diff --git a/netlink/netlink.h b/netlink/netlink.h
index a4984c82ae76..dd4a02bcc916 100644
--- a/netlink/netlink.h
+++ b/netlink/netlink.h
@@ -45,7 +45,7 @@ struct nl_context {
 	const char		*cmd;
 	const char		*param;
 	char			**argp;
-	int			argc;
+	unsigned int		argc;
 	bool			ioctl_fallback;
 	bool			wildcard_unsupported;
 };
diff --git a/netlink/nlsock.c b/netlink/nlsock.c
index c3f09b6ee9ab..ef31d8c33b29 100644
--- a/netlink/nlsock.c
+++ b/netlink/nlsock.c
@@ -168,7 +168,7 @@ static void debug_msg(struct nl_socket *nlsk, const void *msg, unsigned int len,
  *
  * Return: error code extracted from the message
  */
-static int nlsock_process_ack(struct nlmsghdr *nlhdr, ssize_t len,
+static int nlsock_process_ack(struct nlmsghdr *nlhdr, unsigned long len,
 			      unsigned int suppress_nlerr, bool pretty)
 {
 	const struct nlattr *tb[NLMSGERR_ATTR_MAX + 1] = {};
diff --git a/netlink/parser.c b/netlink/parser.c
index 395bd5743af9..c5a368a65a7a 100644
--- a/netlink/parser.c
+++ b/netlink/parser.c
@@ -604,7 +604,7 @@ static int parse_numeric_bitset(struct nl_context *nlctx, uint16_t type,
 		parser_err_invalid_value(nlctx, arg);
 		return -EINVAL;
 	}
-	len1 = maskptr ? (maskptr - arg) : strlen(arg);
+	len1 = maskptr ? (unsigned int)(maskptr - arg) : strlen(arg);
 	nwords = DIV_ROUND_UP(len1, 8);
 	nbits = 0;
 
diff --git a/netlink/settings.c b/netlink/settings.c
index de35ad173627..99d047a3e497 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -276,10 +276,10 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 	const struct nlattr *bitset_tb[ETHTOOL_A_BITSET_MAX + 1] = {};
 	DECLARE_ATTR_TB_INFO(bitset_tb);
 	const unsigned int before_len = strlen(before);
+	unsigned int prev = UINT_MAX - 1;
 	const struct nlattr *bits;
 	const struct nlattr *bit;
 	bool first = true;
-	int prev = -2;
 	bool nomask;
 	int ret;
 
@@ -333,7 +333,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 			if (first)
 				first = false;
 			/* ugly hack to preserve old output format */
-			if (class == LM_CLASS_REAL && (prev == idx - 1) &&
+			if (class == LM_CLASS_REAL && (idx == prev + 1) &&
 			    prev < link_modes_count &&
 			    link_modes[prev].class == LM_CLASS_REAL &&
 			    link_modes[prev].duplex == DUPLEX_HALF)
@@ -375,7 +375,7 @@ int dump_link_modes(struct nl_context *nlctx, const struct nlattr *bitset,
 			first = false;
 		} else {
 			/* ugly hack to preserve old output format */
-			if ((class == LM_CLASS_REAL) && (prev == idx - 1) &&
+			if ((class == LM_CLASS_REAL) && (idx == prev + 1) &&
 			    (prev < link_modes_count) &&
 			    (link_modes[prev].class == LM_CLASS_REAL) &&
 			    (link_modes[prev].duplex == DUPLEX_HALF))
-- 
2.28.0

[PATCH ethtool 2/7] ioctl: check presence of eeprom length argument properly

[Michal Kubecek]

In do_geeprom(), do_seprom() and do_getmodule(), check if user used
"length" command line argument is done by setting the value to -1 before
parsing and checking if it changed. This is quite ugly and also causes
compiler warnings as the variable is u32.

Use proper "seen" flag to let parser tell us if the argument was used.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 ethtool.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index c4ad186cd390..4fa7a2c1716f 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -3184,10 +3184,12 @@ static int do_geeprom(struct cmd_context *ctx)
 	int geeprom_changed = 0;
 	int geeprom_dump_raw = 0;
 	u32 geeprom_offset = 0;
-	u32 geeprom_length = -1;
+	u32 geeprom_length = 0;
+	int geeprom_length_seen = 0;
 	struct cmdline_info cmdline_geeprom[] = {
 		{ "offset", CMDL_U32, &geeprom_offset, NULL },
-		{ "length", CMDL_U32, &geeprom_length, NULL },
+		{ "length", CMDL_U32, &geeprom_length, NULL,
+		  0, &geeprom_length_seen },
 		{ "raw", CMDL_BOOL, &geeprom_dump_raw, NULL },
 	};
 	int err;
@@ -3204,7 +3206,7 @@ static int do_geeprom(struct cmd_context *ctx)
 		return 74;
 	}
 
-	if (geeprom_length == -1)
+	if (!geeprom_length_seen)
 		geeprom_length = drvinfo.eedump_len;
 
 	if (drvinfo.eedump_len < geeprom_offset + geeprom_length)
@@ -3234,14 +3236,16 @@ static int do_seeprom(struct cmd_context *ctx)
 {
 	int seeprom_changed = 0;
 	u32 seeprom_magic = 0;
-	u32 seeprom_length = -1;
+	u32 seeprom_length = 0;
 	u32 seeprom_offset = 0;
 	u8 seeprom_value = 0;
+	int seeprom_length_seen = 0;
 	int seeprom_value_seen = 0;
 	struct cmdline_info cmdline_seeprom[] = {
 		{ "magic", CMDL_U32, &seeprom_magic, NULL },
 		{ "offset", CMDL_U32, &seeprom_offset, NULL },
-		{ "length", CMDL_U32, &seeprom_length, NULL },
+		{ "length", CMDL_U32, &seeprom_length, NULL,
+		  0, &seeprom_length_seen },
 		{ "value", CMDL_U8, &seeprom_value, NULL,
 		  0, &seeprom_value_seen },
 	};
@@ -3262,7 +3266,7 @@ static int do_seeprom(struct cmd_context *ctx)
 	if (seeprom_value_seen)
 		seeprom_length = 1;
 
-	if (seeprom_length == -1)
+	if (!seeprom_length_seen)
 		seeprom_length = drvinfo.eedump_len;
 
 	if (drvinfo.eedump_len < seeprom_offset + seeprom_length) {
@@ -4538,15 +4542,17 @@ static int do_getmodule(struct cmd_context *ctx)
 	struct ethtool_modinfo modinfo;
 	struct ethtool_eeprom *eeprom;
 	u32 geeprom_offset = 0;
-	u32 geeprom_length = -1;
+	u32 geeprom_length = 0;
 	int geeprom_changed = 0;
 	int geeprom_dump_raw = 0;
 	int geeprom_dump_hex = 0;
+	int geeprom_length_seen = 0;
 	int err;
 
 	struct cmdline_info cmdline_geeprom[] = {
 		{ "offset", CMDL_U32, &geeprom_offset, NULL },
-		{ "length", CMDL_U32, &geeprom_length, NULL },
+		{ "length", CMDL_U32, &geeprom_length, NULL,
+		  0, &geeprom_length_seen },
 		{ "raw", CMDL_BOOL, &geeprom_dump_raw, NULL },
 		{ "hex", CMDL_BOOL, &geeprom_dump_hex, NULL },
 	};
@@ -4566,7 +4572,7 @@ static int do_getmodule(struct cmd_context *ctx)
 		return 1;
 	}
 
-	if (geeprom_length == -1)
+	if (!geeprom_length_seen)
 		geeprom_length = modinfo.eeprom_len;
 
 	if (modinfo.eeprom_len < geeprom_offset + geeprom_length)
-- 
2.28.0

[PATCH ethtool 3/7] ioctl: get rid of signed/unsigned comparison warnings

[Michal Kubecek]

Comparison between signed and unsigned values is fragile and causes
compiler warnings with recent compilers and stricter CFLAGS. Prevent such
comparisons either by properly declaring variables (mostly loop iterators)
as unsigned or by explicitly casting one side of the comparison.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 ethtool.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 4fa7a2c1716f..d9dcd0448c02 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -225,8 +225,8 @@ static void parse_generic_cmdline(struct cmd_context *ctx,
 {
 	int argc = ctx->argc;
 	char **argp = ctx->argp;
-	int i, idx;
-	int found;
+	unsigned int idx;
+	int i, found;
 
 	for (i = 0; i < argc; i++) {
 		found = 0;
@@ -641,8 +641,9 @@ static void dump_link_caps(const char *prefix, const char *an_prefix,
 		  "200000baseCR4/Full" },
 	};
 	int indent;
-	int did1, new_line_pend, i;
+	int did1, new_line_pend;
 	int fecreported = 0;
+	unsigned int i;
 
 	/* Indent just like the separate functions used to */
 	indent = strlen(prefix) + 14;
@@ -1071,7 +1072,7 @@ void dump_hex(FILE *file, const u8 *data, int len, int offset)
 static int dump_regs(int gregs_dump_raw, int gregs_dump_hex,
 		     struct ethtool_drvinfo *info, struct ethtool_regs *regs)
 {
-	int i;
+	unsigned int i;
 
 	if (gregs_dump_raw) {
 		fwrite(regs->data, regs->len, 1, stdout);
@@ -1128,7 +1129,8 @@ static int dump_eeprom(int geeprom_dump_raw,
 static int dump_test(struct ethtool_test *test,
 		     struct ethtool_gstrings *strings)
 {
-	int i, rc;
+	unsigned int i;
+	int rc;
 
 	rc = test->flags & ETH_TEST_FL_FAILED;
 	fprintf(stdout, "The test result is %s\n", rc ? "FAIL" : "PASS");
@@ -1359,7 +1361,7 @@ static void dump_one_feature(const char *indent, const char *name,
 	       : "");
 }
 
-static int linux_version_code(void)
+static unsigned int linux_version_code(void)
 {
 	struct utsname utsname;
 	unsigned version, patchlevel, sublevel = 0;
@@ -1375,10 +1377,10 @@ static void dump_features(const struct feature_defs *defs,
 			  const struct feature_state *state,
 			  const struct feature_state *ref_state)
 {
-	int kernel_ver = linux_version_code();
-	u32 value;
+	unsigned int kernel_ver = linux_version_code();
+	unsigned int i, j;
 	int indent;
-	int i, j;
+	u32 value;
 
 	for (i = 0; i < OFF_FLAG_DEF_SIZE; i++) {
 		/* Don't show features whose state is unknown on this
@@ -1411,7 +1413,7 @@ static void dump_features(const struct feature_defs *defs,
 
 		/* Show matching features */
 		for (j = 0; j < defs->n_features; j++) {
-			if (defs->def[j].off_flag_index != i)
+			if (defs->def[j].off_flag_index != (int)i)
 				continue;
 			if (defs->off_flag_matched[i] != 1)
 				/* Show all matching feature states */
@@ -1668,8 +1670,8 @@ static struct feature_defs *get_feature_defs(struct cmd_context *ctx)
 {
 	struct ethtool_gstrings *names;
 	struct feature_defs *defs;
+	unsigned int i, j;
 	u32 n_features;
-	int i, j;
 
 	names = get_stringset(ctx, ETH_SS_FEATURES, 0, 1);
 	if (names) {
@@ -2236,8 +2238,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 	struct cmdline_info *cmdline_features;
 	struct feature_state *old_state, *new_state;
 	struct ethtool_value eval;
+	unsigned int i, j;
 	int err, rc;
-	int i, j;
 
 	defs = get_feature_defs(ctx);
 	if (!defs) {
@@ -2317,7 +2319,7 @@ static int do_sfeatures(struct cmd_context *ctx)
 				continue;
 
 			for (j = 0; j < defs->n_features; j++) {
-				if (defs->def[j].off_flag_index != i ||
+				if (defs->def[j].off_flag_index != (int)i ||
 				    !FEATURE_BIT_IS_SET(
 					    old_state->features.features,
 					    j, available) ||
@@ -2724,9 +2726,9 @@ static int do_sset(struct cmd_context *ctx)
 	u32 msglvl_wanted = 0;
 	u32 msglvl_mask = 0;
 	struct cmdline_info cmdline_msglvl[n_flags_msglvl];
-	int argc = ctx->argc;
+	unsigned int argc = ctx->argc;
 	char **argp = ctx->argp;
-	int i;
+	unsigned int i;
 	int err = 0;
 
 	for (i = 0; i < n_flags_msglvl; i++)
@@ -3869,7 +3871,7 @@ static int do_srxfh(struct cmd_context *ctx)
 	char *hfunc_name = NULL;
 	char *hkey = NULL;
 	int err = 0;
-	int i;
+	unsigned int i;
 	u32 arg_num = 0, indir_bytes = 0;
 	u32 req_hfunc = 0;
 	u32 entry_size = sizeof(rss_head.rss_config[0]);
@@ -3880,7 +3882,7 @@ static int do_srxfh(struct cmd_context *ctx)
 	if (ctx->argc < 1)
 		exit_bad_args();
 
-	while (arg_num < ctx->argc) {
+	while (arg_num < (unsigned int)ctx->argc) {
 		if (!strcmp(ctx->argp[arg_num], "equal")) {
 			++arg_num;
 			rxfhindir_equal = get_int_range(ctx->argp[arg_num],
@@ -3894,7 +3896,7 @@ static int do_srxfh(struct cmd_context *ctx)
 		} else if (!strcmp(ctx->argp[arg_num], "weight")) {
 			++arg_num;
 			rxfhindir_weight = ctx->argp + arg_num;
-			while (arg_num < ctx->argc &&
+			while (arg_num < (unsigned int)ctx->argc &&
 			       isdigit((unsigned char)ctx->argp[arg_num][0])) {
 				++arg_num;
 				++num_weights;
@@ -4135,7 +4137,8 @@ static int do_flash(struct cmd_context *ctx)
 
 static int do_permaddr(struct cmd_context *ctx)
 {
-	int i, err;
+	unsigned int i;
+	int err;
 	struct ethtool_perm_addr *epaddr;
 
 	epaddr = malloc(sizeof(struct ethtool_perm_addr) + MAX_ADDR_LEN);
@@ -4750,7 +4753,7 @@ static int do_stunable(struct cmd_context *ctx)
 	struct cmdline_info cmdline_tunable[TUNABLES_INFO_SIZE];
 	struct ethtool_tunable_info *tinfo = tunables_info;
 	int changed = 0;
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < TUNABLES_INFO_SIZE; i++) {
 		cmdline_tunable[i].name = tunable_strings[tinfo[i].t_id];
@@ -4833,8 +4836,8 @@ static int do_gtunable(struct cmd_context *ctx)
 	struct ethtool_tunable_info *tinfo = tunables_info;
 	char **argp = ctx->argp;
 	int argc = ctx->argc;
+	unsigned int j;
 	int i;
-	int j;
 
 	if (argc < 1)
 		exit_bad_args();
-- 
2.28.0

[PATCH ethtool 4/7] get rid of signed/unsigned comparison warnings in register dump parsers

[Michal Kubecek]

All of these are avoided by declaring a variable (mostly loop iterators)
holding only unsigned values as unsigned.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 dsa.c      | 2 +-
 fec.c      | 2 +-
 ibm_emac.c | 2 +-
 marvell.c  | 2 +-
 natsemi.c  | 2 +-
 rxclass.c  | 8 +++++---
 sfpdiag.c  | 2 +-
 tg3.c      | 4 ++--
 8 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/dsa.c b/dsa.c
index 65502a899194..33c1d39d6605 100644
--- a/dsa.c
+++ b/dsa.c
@@ -824,8 +824,8 @@ static int dsa_mv88e6xxx_dump_regs(struct ethtool_regs *regs)
 {
 	const struct dsa_mv88e6xxx_switch *sw = NULL;
 	const u16 *data = (u16 *)regs->data;
+	unsigned int i;
 	u16 id;
-	int i;
 
 	/* Marvell chips have 32 per-port 16-bit registers */
 	if (regs->len < 32 * sizeof(u16))
diff --git a/fec.c b/fec.c
index 9cb4f8b1d4e1..d2373d6124c0 100644
--- a/fec.c
+++ b/fec.c
@@ -198,7 +198,7 @@ int fec_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		  struct ethtool_regs *regs)
 {
 	const u32 *data = (u32 *)regs->data;
-	int offset;
+	unsigned int offset;
 	u32 val;
 
 	for (offset = 0; offset < regs->len; offset += 4) {
diff --git a/ibm_emac.c b/ibm_emac.c
index ea01d56f609c..9f7cae605482 100644
--- a/ibm_emac.c
+++ b/ibm_emac.c
@@ -238,7 +238,7 @@ static void *print_mal_regs(void *buf)
 {
 	struct emac_ethtool_regs_subhdr *hdr = buf;
 	struct mal_regs *p = (struct mal_regs *)(hdr + 1);
-	int i;
+	unsigned int i;
 
 	printf("MAL%d Registers\n", hdr->index);
 	printf("-----------------\n");
diff --git a/marvell.c b/marvell.c
index 8afb150327a3..d3d570e4d4ad 100644
--- a/marvell.c
+++ b/marvell.c
@@ -130,7 +130,7 @@ static void dump_fifo(const char *name, const void *p)
 static void dump_gmac_fifo(const char *name, const void *p)
 {
 	const u32 *r = p;
-	int i;
+	unsigned int i;
 	static const char *regs[] = {
 		"End Address",
 		"Almost Full Thresh",
diff --git a/natsemi.c b/natsemi.c
index 0af465959cbc..4d9fc092b623 100644
--- a/natsemi.c
+++ b/natsemi.c
@@ -967,8 +967,8 @@ int
 natsemi_dump_eeprom(struct ethtool_drvinfo *info __maybe_unused,
 		    struct ethtool_eeprom *ee)
 {
-	int i;
 	u16 *eebuf = (u16 *)ee->data;
+	unsigned int i;
 
 	if (ee->magic != NATSEMI_MAGIC) {
 		fprintf(stderr, "Magic number 0x%08x does not match 0x%08x\n",
diff --git a/rxclass.c b/rxclass.c
index 79972651e706..6cf81fdafc85 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -348,8 +348,9 @@ int rxclass_rule_getall(struct cmd_context *ctx)
 {
 	struct ethtool_rxnfc *nfccmd;
 	__u32 *rule_locs;
-	int err, i;
+	unsigned int i;
 	__u32 count;
+	int err;
 
 	/* determine rule count */
 	err = rxclass_get_dev_info(ctx, &count, NULL);
@@ -481,8 +482,9 @@ static int rmgr_find_empty_slot(struct rmgr_ctrl *rmgr,
 static int rmgr_init(struct cmd_context *ctx, struct rmgr_ctrl *rmgr)
 {
 	struct ethtool_rxnfc *nfccmd;
-	int err, i;
 	__u32 *rule_locs;
+	unsigned int i;
+	int err;
 
 	/* clear rule manager settings */
 	memset(rmgr, 0, sizeof(*rmgr));
@@ -941,7 +943,7 @@ static int rxclass_get_long(char *str, long long *val, int size)
 
 static int rxclass_get_ulong(char *str, unsigned long long *val, int size)
 {
-	long long max = ~0ULL >> (64 - size);
+	unsigned long long max = ~0ULL >> (64 - size);
 	char *endp;
 
 	errno = 0;
diff --git a/sfpdiag.c b/sfpdiag.c
index fa41651422ea..1fa8b7ba8fec 100644
--- a/sfpdiag.c
+++ b/sfpdiag.c
@@ -190,8 +190,8 @@ static float befloattoh(const __u32 *source)
 
 static void sff8472_calibration(const __u8 *id, struct sff_diags *sd)
 {
-	int i;
 	__u16 rx_reading;
+	unsigned int i;
 
 	/* Calibration should occur for all values (threshold and current) */
 	for (i = 0; i < ARRAY_SIZE(sd->bias_cur); ++i) {
diff --git a/tg3.c b/tg3.c
index ac73b33ae4e3..ebdef2d60e6b 100644
--- a/tg3.c
+++ b/tg3.c
@@ -7,7 +7,7 @@
 int tg3_dump_eeprom(struct ethtool_drvinfo *info __maybe_unused,
 		    struct ethtool_eeprom *ee)
 {
-	int i;
+	unsigned int i;
 
 	if (ee->magic != TG3_MAGIC) {
 		fprintf(stderr, "Magic number 0x%08x does not match 0x%08x\n",
@@ -26,7 +26,7 @@ int tg3_dump_eeprom(struct ethtool_drvinfo *info __maybe_unused,
 int tg3_dump_regs(struct ethtool_drvinfo *info __maybe_unused,
 		  struct ethtool_regs *regs)
 {
-	int i;
+	unsigned int i;
 	u32 reg;
 
 	fprintf(stdout, "Offset\tValue\n");
-- 
2.28.0

[PATCH ethtool 5/7] settings: simplify link_mode_info[] initializers

[Michal Kubecek]

Use macro helpers to make link_mode_info[] initializers easier to read and
less prone to mistakes. As a bonus, this gets rid of "missing field
initializer" warnings in netlink/settings.c

This commit should have no effect on resulting code (checked with gcc-11
and -O2).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 netlink/settings.c | 236 +++++++++++++++++----------------------------
 1 file changed, 86 insertions(+), 150 deletions(-)

diff --git a/netlink/settings.c b/netlink/settings.c
index 99d047a3e497..935724e799da 100644
--- a/netlink/settings.c
+++ b/netlink/settings.c
@@ -64,160 +64,96 @@ static const char *const names_transceiver[] = {
  * there is little chance of getting them separated any time soon so let's
  * sort them out ourselves
  */
+#define __REAL(_speed) \
+	{ .class = LM_CLASS_REAL, .speed = _speed, .duplex = DUPLEX_FULL }
+#define __HALF_DUPLEX(_speed) \
+	{ .class = LM_CLASS_REAL, .speed = _speed, .duplex = DUPLEX_HALF }
+#define __SPECIAL(_class) \
+	{ .class = LM_CLASS_ ## _class }
+
 static const struct link_mode_info link_modes[] = {
-	[ETHTOOL_LINK_MODE_10baseT_Half_BIT] =
-		{ LM_CLASS_REAL,	10,	DUPLEX_HALF },
-	[ETHTOOL_LINK_MODE_10baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	10,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100baseT_Half_BIT] =
-		{ LM_CLASS_REAL,	100,	DUPLEX_HALF },
-	[ETHTOOL_LINK_MODE_100baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	100,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_1000baseT_Half_BIT] =
-		{ LM_CLASS_REAL,	1000,	DUPLEX_HALF },
-	[ETHTOOL_LINK_MODE_1000baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	1000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_Autoneg_BIT] =
-		{ LM_CLASS_AUTONEG },
-	[ETHTOOL_LINK_MODE_TP_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_AUI_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_MII_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_FIBRE_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_BNC_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_10000baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_Pause_BIT] =
-		{ LM_CLASS_PAUSE },
-	[ETHTOOL_LINK_MODE_Asym_Pause_BIT] =
-		{ LM_CLASS_PAUSE },
-	[ETHTOOL_LINK_MODE_2500baseX_Full_BIT] =
-		{ LM_CLASS_REAL,	2500,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_Backplane_BIT] =
-		{ LM_CLASS_PORT },
-	[ETHTOOL_LINK_MODE_1000baseKX_Full_BIT] =
-		{ LM_CLASS_REAL,	1000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseKR_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT] =
-		{ LM_CLASS_REAL,	20000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT] =
-		{ LM_CLASS_REAL,	20000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT] =
-		{ LM_CLASS_REAL,	40000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT] =
-		{ LM_CLASS_REAL,	40000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT] =
-		{ LM_CLASS_REAL,	40000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT] =
-		{ LM_CLASS_REAL,	40000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT] =
-		{ LM_CLASS_REAL,	56000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT] =
-		{ LM_CLASS_REAL,	56000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT] =
-		{ LM_CLASS_REAL,	56000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT] =
-		{ LM_CLASS_REAL,	56000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_25000baseCR_Full_BIT] =
-		{ LM_CLASS_REAL,	25000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_25000baseKR_Full_BIT] =
-		{ LM_CLASS_REAL,	25000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_25000baseSR_Full_BIT] =
-		{ LM_CLASS_REAL,	25000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_1000baseX_Full_BIT] =
-		{ LM_CLASS_REAL,	1000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseCR_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseSR_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseLR_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_10000baseER_Full_BIT] =
-		{ LM_CLASS_REAL,	10000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_2500baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	2500,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_5000baseT_Full_BIT] =
-		{ LM_CLASS_REAL,	5000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_FEC_NONE_BIT] =
-		{ LM_CLASS_FEC },
-	[ETHTOOL_LINK_MODE_FEC_RS_BIT] =
-		{ LM_CLASS_FEC },
-	[ETHTOOL_LINK_MODE_FEC_BASER_BIT] =
-		{ LM_CLASS_FEC },
-	[ETHTOOL_LINK_MODE_50000baseKR_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseSR_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseCR_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_50000baseDR_Full_BIT] =
-		{ LM_CLASS_REAL,	50000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT] =
-		{ LM_CLASS_REAL,	100000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT] =
-		{ LM_CLASS_REAL,	200000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT] =
-		{ LM_CLASS_REAL,	200000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT] =
-		{ LM_CLASS_REAL,	200000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT] =
-		{ LM_CLASS_REAL,	200000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT] =
-		{ LM_CLASS_REAL,	200000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_100baseT1_Full_BIT] =
-		{ LM_CLASS_REAL,	100,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_1000baseT1_Full_BIT] =
-		{ LM_CLASS_REAL,	1000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_400000baseKR8_Full_BIT] =
-		{ LM_CLASS_REAL,	400000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_400000baseSR8_Full_BIT] =
-		{ LM_CLASS_REAL,	400000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_400000baseLR8_ER8_FR8_Full_BIT] =
-		{ LM_CLASS_REAL,	400000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_400000baseDR8_Full_BIT] =
-		{ LM_CLASS_REAL,	400000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_400000baseCR8_Full_BIT] =
-		{ LM_CLASS_REAL,	400000,	DUPLEX_FULL },
-	[ETHTOOL_LINK_MODE_FEC_LLRS_BIT] =
-		{ LM_CLASS_FEC },
+	[ETHTOOL_LINK_MODE_10baseT_Half_BIT]		= __HALF_DUPLEX(10),
+	[ETHTOOL_LINK_MODE_10baseT_Full_BIT]		= __REAL(10),
+	[ETHTOOL_LINK_MODE_100baseT_Half_BIT]		= __HALF_DUPLEX(100),
+	[ETHTOOL_LINK_MODE_100baseT_Full_BIT]		= __REAL(100),
+	[ETHTOOL_LINK_MODE_1000baseT_Half_BIT]		= __HALF_DUPLEX(1000),
+	[ETHTOOL_LINK_MODE_1000baseT_Full_BIT]		= __REAL(1000),
+	[ETHTOOL_LINK_MODE_Autoneg_BIT]			= __SPECIAL(AUTONEG),
+	[ETHTOOL_LINK_MODE_TP_BIT]			= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_AUI_BIT]			= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_MII_BIT]			= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_FIBRE_BIT]			= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_BNC_BIT]			= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_10000baseT_Full_BIT]		= __REAL(10000),
+	[ETHTOOL_LINK_MODE_Pause_BIT]			= __SPECIAL(PAUSE),
+	[ETHTOOL_LINK_MODE_Asym_Pause_BIT]		= __SPECIAL(PAUSE),
+	[ETHTOOL_LINK_MODE_2500baseX_Full_BIT]		= __REAL(2500),
+	[ETHTOOL_LINK_MODE_Backplane_BIT]		= __SPECIAL(PORT),
+	[ETHTOOL_LINK_MODE_1000baseKX_Full_BIT]		= __REAL(1000),
+	[ETHTOOL_LINK_MODE_10000baseKX4_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseKR_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseR_FEC_BIT]		= __REAL(10000),
+	[ETHTOOL_LINK_MODE_20000baseMLD2_Full_BIT]	= __REAL(20000),
+	[ETHTOOL_LINK_MODE_20000baseKR2_Full_BIT]	= __REAL(20000),
+	[ETHTOOL_LINK_MODE_40000baseKR4_Full_BIT]	= __REAL(40000),
+	[ETHTOOL_LINK_MODE_40000baseCR4_Full_BIT]	= __REAL(40000),
+	[ETHTOOL_LINK_MODE_40000baseSR4_Full_BIT]	= __REAL(40000),
+	[ETHTOOL_LINK_MODE_40000baseLR4_Full_BIT]	= __REAL(40000),
+	[ETHTOOL_LINK_MODE_56000baseKR4_Full_BIT]	= __REAL(56000),
+	[ETHTOOL_LINK_MODE_56000baseCR4_Full_BIT]	= __REAL(56000),
+	[ETHTOOL_LINK_MODE_56000baseSR4_Full_BIT]	= __REAL(56000),
+	[ETHTOOL_LINK_MODE_56000baseLR4_Full_BIT]	= __REAL(56000),
+	[ETHTOOL_LINK_MODE_25000baseCR_Full_BIT]	= __REAL(25000),
+	[ETHTOOL_LINK_MODE_25000baseKR_Full_BIT]	= __REAL(25000),
+	[ETHTOOL_LINK_MODE_25000baseSR_Full_BIT]	= __REAL(25000),
+	[ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_50000baseKR2_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseSR4_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseCR4_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseLR4_ER4_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_50000baseSR2_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_1000baseX_Full_BIT]		= __REAL(1000),
+	[ETHTOOL_LINK_MODE_10000baseCR_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseSR_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseLR_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseLRM_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_10000baseER_Full_BIT]	= __REAL(10000),
+	[ETHTOOL_LINK_MODE_2500baseT_Full_BIT]		= __REAL(2500),
+	[ETHTOOL_LINK_MODE_5000baseT_Full_BIT]		= __REAL(5000),
+	[ETHTOOL_LINK_MODE_FEC_NONE_BIT]		= __SPECIAL(FEC),
+	[ETHTOOL_LINK_MODE_FEC_RS_BIT]			= __SPECIAL(FEC),
+	[ETHTOOL_LINK_MODE_FEC_BASER_BIT]		= __SPECIAL(FEC),
+	[ETHTOOL_LINK_MODE_50000baseKR_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_50000baseSR_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_50000baseCR_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_50000baseLR_ER_FR_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_50000baseDR_Full_BIT]	= __REAL(50000),
+	[ETHTOOL_LINK_MODE_100000baseKR2_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseSR2_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseCR2_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseLR2_ER2_FR2_Full_BIT] = __REAL(100000),
+	[ETHTOOL_LINK_MODE_100000baseDR2_Full_BIT]	= __REAL(100000),
+	[ETHTOOL_LINK_MODE_200000baseKR4_Full_BIT]	= __REAL(200000),
+	[ETHTOOL_LINK_MODE_200000baseSR4_Full_BIT]	= __REAL(200000),
+	[ETHTOOL_LINK_MODE_200000baseLR4_ER4_FR4_Full_BIT] = __REAL(200000),
+	[ETHTOOL_LINK_MODE_200000baseDR4_Full_BIT]	= __REAL(200000),
+	[ETHTOOL_LINK_MODE_200000baseCR4_Full_BIT]	= __REAL(200000),
+	[ETHTOOL_LINK_MODE_100baseT1_Full_BIT]		= __REAL(100),
+	[ETHTOOL_LINK_MODE_1000baseT1_Full_BIT]		= __REAL(1000),
+	[ETHTOOL_LINK_MODE_400000baseKR8_Full_BIT]	= __REAL(400000),
+	[ETHTOOL_LINK_MODE_400000baseSR8_Full_BIT]	= __REAL(400000),
+	[ETHTOOL_LINK_MODE_400000baseLR8_ER8_FR8_Full_BIT] = __REAL(400000),
+	[ETHTOOL_LINK_MODE_400000baseDR8_Full_BIT]	= __REAL(400000),
+	[ETHTOOL_LINK_MODE_400000baseCR8_Full_BIT]	= __REAL(400000),
+	[ETHTOOL_LINK_MODE_FEC_LLRS_BIT]		= __SPECIAL(FEC),
 };
 const unsigned int link_modes_count = ARRAY_SIZE(link_modes);
 
+#undef __REAL
+#undef __HALF_DUPLEX
+#undef __SPECIAL
+
 static bool lm_class_match(unsigned int mode, enum link_mode_class class)
 {
 	unsigned int mode_class = (mode < link_modes_count) ?
-- 
2.28.0

[PATCH ethtool 6/7] ioctl: convert cmdline_info arrays to named initializers

[Michal Kubecek]

To get rid of remaining "missing field initializer" compiler warnings,
convert arrays of struct cmdline_info used for command line parser to
named initializers. This also makes the initializers easier to read.

This commit should have no effect on resulting code (checked with gcc-11
and -O2).

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 ethtool.c | 378 ++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 296 insertions(+), 82 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index d9dcd0448c02..40868d064e28 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1825,10 +1825,24 @@ static int do_spause(struct cmd_context *ctx)
 	int pause_rx_wanted = -1;
 	int pause_tx_wanted = -1;
 	struct cmdline_info cmdline_pause[] = {
-		{ "autoneg", CMDL_BOOL, &pause_autoneg_wanted,
-		  &epause.autoneg },
-		{ "rx", CMDL_BOOL, &pause_rx_wanted, &epause.rx_pause },
-		{ "tx", CMDL_BOOL, &pause_tx_wanted, &epause.tx_pause },
+		{
+			.name		= "autoneg",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &pause_autoneg_wanted,
+			.ioctl_val	= &epause.autoneg,
+		},
+		{
+			.name		= "rx",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &pause_rx_wanted,
+			.ioctl_val	= &epause.rx_pause,
+		},
+		{
+			.name		= "tx",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &pause_tx_wanted,
+			.ioctl_val	= &epause.tx_pause,
+		},
 	};
 	int err, changed = 0;
 
@@ -1868,12 +1882,30 @@ static int do_sring(struct cmd_context *ctx)
 	s32 ring_rx_jumbo_wanted = -1;
 	s32 ring_tx_wanted = -1;
 	struct cmdline_info cmdline_ring[] = {
-		{ "rx", CMDL_S32, &ring_rx_wanted, &ering.rx_pending },
-		{ "rx-mini", CMDL_S32, &ring_rx_mini_wanted,
-		  &ering.rx_mini_pending },
-		{ "rx-jumbo", CMDL_S32, &ring_rx_jumbo_wanted,
-		  &ering.rx_jumbo_pending },
-		{ "tx", CMDL_S32, &ring_tx_wanted, &ering.tx_pending },
+		{
+			.name		= "rx",
+			.type		= CMDL_S32,
+			.wanted_val	= &ring_rx_wanted,
+			.ioctl_val	= &ering.rx_pending,
+		},
+		{
+			.name		= "rx-mini",
+			.type		= CMDL_S32,
+			.wanted_val	= &ring_rx_mini_wanted,
+			.ioctl_val	= &ering.rx_mini_pending,
+		},
+		{
+			.name		= "rx-jumbo",
+			.type		= CMDL_S32,
+			.wanted_val	= &ring_rx_jumbo_wanted,
+			.ioctl_val	= &ering.rx_jumbo_pending,
+		},
+		{
+			.name		= "tx",
+			.type		= CMDL_S32,
+			.wanted_val	= &ring_tx_wanted,
+			.ioctl_val	= &ering.tx_pending,
+		},
 	};
 	int err, changed = 0;
 
@@ -1937,12 +1969,30 @@ static int do_schannels(struct cmd_context *ctx)
 	s32 channels_other_wanted = -1;
 	s32 channels_combined_wanted = -1;
 	struct cmdline_info cmdline_channels[] = {
-		{ "rx", CMDL_S32, &channels_rx_wanted, &echannels.rx_count },
-		{ "tx", CMDL_S32, &channels_tx_wanted, &echannels.tx_count },
-		{ "other", CMDL_S32, &channels_other_wanted,
-		  &echannels.other_count },
-		{ "combined", CMDL_S32, &channels_combined_wanted,
-		  &echannels.combined_count },
+		{
+			.name		= "rx",
+			.type		= CMDL_S32,
+			.wanted_val	= &channels_rx_wanted,
+			.ioctl_val	= &echannels.rx_count,
+		},
+		{
+			.name		= "tx",
+			.type		= CMDL_S32,
+			.wanted_val	= &channels_tx_wanted,
+			.ioctl_val	= &echannels.tx_count,
+		},
+		{
+			.name		= "other",
+			.type		= CMDL_S32,
+			.wanted_val	= &channels_other_wanted,
+			.ioctl_val	= &echannels.other_count,
+		},
+		{
+			.name		= "combined",
+			.type		= CMDL_S32,
+			.wanted_val	= &channels_combined_wanted,
+			.ioctl_val	= &echannels.combined_count,
+		},
 	};
 	int err, changed = 0;
 
@@ -2052,50 +2102,138 @@ static int do_gcoalesce(struct cmd_context *ctx)
 
 #define COALESCE_CMDLINE_INFO(__ecoal)					\
 {									\
-	{ "adaptive-rx", CMDL_BOOL, &coal_adaptive_rx_wanted,		\
-	  &__ecoal.use_adaptive_rx_coalesce },				\
-	{ "adaptive-tx", CMDL_BOOL, &coal_adaptive_tx_wanted,		\
-	  &__ecoal.use_adaptive_tx_coalesce },				\
-	{ "sample-interval", CMDL_S32, &coal_sample_rate_wanted,	\
-	  &__ecoal.rate_sample_interval },				\
-	{ "stats-block-usecs", CMDL_S32, &coal_stats_wanted,		\
-	  &__ecoal.stats_block_coalesce_usecs },			\
-	{ "pkt-rate-low", CMDL_S32, &coal_pkt_rate_low_wanted,		\
-	  &__ecoal.pkt_rate_low },					\
-	{ "pkt-rate-high", CMDL_S32, &coal_pkt_rate_high_wanted,	\
-	  &__ecoal.pkt_rate_high },					\
-	{ "rx-usecs", CMDL_S32, &coal_rx_usec_wanted,			\
-	  &__ecoal.rx_coalesce_usecs },					\
-	{ "rx-frames", CMDL_S32, &coal_rx_frames_wanted,		\
-	  &__ecoal.rx_max_coalesced_frames },				\
-	{ "rx-usecs-irq", CMDL_S32, &coal_rx_usec_irq_wanted,		\
-	  &__ecoal.rx_coalesce_usecs_irq },				\
-	{ "rx-frames-irq", CMDL_S32, &coal_rx_frames_irq_wanted,	\
-	  &__ecoal.rx_max_coalesced_frames_irq },			\
-	{ "tx-usecs", CMDL_S32, &coal_tx_usec_wanted,			\
-	  &__ecoal.tx_coalesce_usecs },					\
-	{ "tx-frames", CMDL_S32, &coal_tx_frames_wanted,		\
-	  &__ecoal.tx_max_coalesced_frames },				\
-	{ "tx-usecs-irq", CMDL_S32, &coal_tx_usec_irq_wanted,		\
-	  &__ecoal.tx_coalesce_usecs_irq },				\
-	{ "tx-frames-irq", CMDL_S32, &coal_tx_frames_irq_wanted,	\
-	  &__ecoal.tx_max_coalesced_frames_irq },			\
-	{ "rx-usecs-low", CMDL_S32, &coal_rx_usec_low_wanted,		\
-	  &__ecoal.rx_coalesce_usecs_low },				\
-	{ "rx-frames-low", CMDL_S32, &coal_rx_frames_low_wanted,	\
-	  &__ecoal.rx_max_coalesced_frames_low },			\
-	{ "tx-usecs-low", CMDL_S32, &coal_tx_usec_low_wanted,		\
-	  &__ecoal.tx_coalesce_usecs_low },				\
-	{ "tx-frames-low", CMDL_S32, &coal_tx_frames_low_wanted,	\
-	  &__ecoal.tx_max_coalesced_frames_low },			\
-	{ "rx-usecs-high", CMDL_S32, &coal_rx_usec_high_wanted,		\
-	  &__ecoal.rx_coalesce_usecs_high },				\
-	{ "rx-frames-high", CMDL_S32, &coal_rx_frames_high_wanted,	\
-	  &__ecoal.rx_max_coalesced_frames_high },			\
-	{ "tx-usecs-high", CMDL_S32, &coal_tx_usec_high_wanted,		\
-	  &__ecoal.tx_coalesce_usecs_high },				\
-	{ "tx-frames-high", CMDL_S32, &coal_tx_frames_high_wanted,	\
-	  &__ecoal.tx_max_coalesced_frames_high },			\
+	{								\
+		.name		= "adaptive-rx",			\
+		.type		= CMDL_BOOL,				\
+		.wanted_val	= &coal_adaptive_rx_wanted,		\
+		.ioctl_val	= &__ecoal.use_adaptive_rx_coalesce,	\
+	},								\
+	{								\
+		.name		= "adaptive-tx",			\
+		.type		= CMDL_BOOL,				\
+		.wanted_val	= &coal_adaptive_tx_wanted,		\
+		.ioctl_val	= &__ecoal.use_adaptive_tx_coalesce,	\
+	},								\
+	{								\
+		.name		= "sample-interval",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_sample_rate_wanted,		\
+		.ioctl_val	= &__ecoal.rate_sample_interval,	\
+	},								\
+	{								\
+		.name		= "stats-block-usecs",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_stats_wanted,			\
+		.ioctl_val	= &__ecoal.stats_block_coalesce_usecs,	\
+	},								\
+	{								\
+		.name		= "pkt-rate-low",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_pkt_rate_low_wanted,		\
+		.ioctl_val	= &__ecoal.pkt_rate_low,		\
+	},								\
+	{								\
+		.name		= "pkt-rate-high",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_pkt_rate_high_wanted,		\
+		.ioctl_val	= &__ecoal.pkt_rate_high,		\
+	},								\
+	{								\
+		.name		= "rx-usecs",				\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_usec_wanted,			\
+		.ioctl_val	= &__ecoal.rx_coalesce_usecs,		\
+	},								\
+	{								\
+		.name		= "rx-frames",				\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_frames_wanted,		\
+		.ioctl_val	= &__ecoal.rx_max_coalesced_frames,	\
+	},								\
+	{								\
+		.name		= "rx-usecs-irq",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_usec_irq_wanted,		\
+		.ioctl_val	= &__ecoal.rx_coalesce_usecs_irq,	\
+	},								\
+	{								\
+		.name		= "rx-frames-irq",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_frames_irq_wanted,		\
+		.ioctl_val	= &__ecoal.rx_max_coalesced_frames_irq,	\
+	},								\
+	{								\
+		.name		= "tx-usecs",				\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_usec_wanted,			\
+		.ioctl_val	= &__ecoal.tx_coalesce_usecs,		\
+	},								\
+	{								\
+		.name		= "tx-frames",				\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_frames_wanted,		\
+		.ioctl_val	= &__ecoal.tx_max_coalesced_frames,	\
+	},								\
+	{								\
+		.name		= "tx-usecs-irq",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_usec_irq_wanted,		\
+		.ioctl_val	= &__ecoal.tx_coalesce_usecs_irq,	\
+	},								\
+	{								\
+		.name		= "tx-frames-irq",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_frames_irq_wanted,		\
+		.ioctl_val	= &__ecoal.tx_max_coalesced_frames_irq,	\
+	},								\
+	{								\
+		.name		= "rx-usecs-low",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_usec_low_wanted,		\
+		.ioctl_val	= &__ecoal.rx_coalesce_usecs_low,	\
+	},								\
+	{								\
+		.name		= "rx-frames-low",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_frames_low_wanted,		\
+		.ioctl_val	= &__ecoal.rx_max_coalesced_frames_low,	\
+	},								\
+	{								\
+		.name		= "tx-usecs-low",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_usec_low_wanted,		\
+		.ioctl_val	= &__ecoal.tx_coalesce_usecs_low,	\
+	},								\
+	{								\
+		.name		= "tx-frames-low",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_frames_low_wanted,		\
+		.ioctl_val	= &__ecoal.tx_max_coalesced_frames_low,	\
+	},								\
+	{								\
+		.name		= "rx-usecs-high",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_usec_high_wanted,		\
+		.ioctl_val	= &__ecoal.rx_coalesce_usecs_high,	\
+	},								\
+	{								\
+		.name		= "rx-frames-high",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_rx_frames_high_wanted,		\
+		.ioctl_val	= &__ecoal.rx_max_coalesced_frames_high,\
+	},								\
+	{								\
+		.name		= "tx-usecs-high",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_usec_high_wanted,		\
+		.ioctl_val	= &__ecoal.tx_coalesce_usecs_high,	\
+	},								\
+	{								\
+		.name		= "tx-frames-high",			\
+		.type		= CMDL_S32,				\
+		.wanted_val	= &coal_tx_frames_high_wanted,		\
+		.ioctl_val	= &__ecoal.tx_max_coalesced_frames_high,\
+	},								\
 }
 
 static int do_scoalesce(struct cmd_context *ctx)
@@ -3090,9 +3228,21 @@ static int do_gregs(struct cmd_context *ctx)
 	int gregs_dump_hex = 0;
 	char *gregs_dump_file = NULL;
 	struct cmdline_info cmdline_gregs[] = {
-		{ "raw", CMDL_BOOL, &gregs_dump_raw, NULL },
-		{ "hex", CMDL_BOOL, &gregs_dump_hex, NULL },
-		{ "file", CMDL_STR, &gregs_dump_file, NULL },
+		{
+			.name		= "raw",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &gregs_dump_raw,
+		},
+		{
+			.name		= "hex",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &gregs_dump_hex,
+		},
+		{
+			.name		= "file",
+			.type		= CMDL_STR,
+			.wanted_val	= &gregs_dump_file,
+		},
 	};
 	int err;
 	struct ethtool_drvinfo drvinfo;
@@ -3189,10 +3339,22 @@ static int do_geeprom(struct cmd_context *ctx)
 	u32 geeprom_length = 0;
 	int geeprom_length_seen = 0;
 	struct cmdline_info cmdline_geeprom[] = {
-		{ "offset", CMDL_U32, &geeprom_offset, NULL },
-		{ "length", CMDL_U32, &geeprom_length, NULL,
-		  0, &geeprom_length_seen },
-		{ "raw", CMDL_BOOL, &geeprom_dump_raw, NULL },
+		{
+			.name		= "offset",
+			.type		= CMDL_U32,
+			.wanted_val	= &geeprom_offset,
+		},
+		{
+			.name		= "length",
+			.type		= CMDL_U32,
+			.wanted_val	= &geeprom_length,
+			.seen_val	= &geeprom_length_seen,
+		},
+		{
+			.name		= "raw",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &geeprom_dump_raw,
+		},
 	};
 	int err;
 	struct ethtool_drvinfo drvinfo;
@@ -3244,12 +3406,28 @@ static int do_seeprom(struct cmd_context *ctx)
 	int seeprom_length_seen = 0;
 	int seeprom_value_seen = 0;
 	struct cmdline_info cmdline_seeprom[] = {
-		{ "magic", CMDL_U32, &seeprom_magic, NULL },
-		{ "offset", CMDL_U32, &seeprom_offset, NULL },
-		{ "length", CMDL_U32, &seeprom_length, NULL,
-		  0, &seeprom_length_seen },
-		{ "value", CMDL_U8, &seeprom_value, NULL,
-		  0, &seeprom_value_seen },
+		{
+			.name		= "magic",
+			.type		= CMDL_U32,
+			.wanted_val	= &seeprom_magic,
+		},
+		{
+			.name		= "offset",
+			.type		= CMDL_U32,
+			.wanted_val	= &seeprom_offset,
+		},
+		{
+			.name		= "length",
+			.type		= CMDL_U32,
+			.wanted_val	= &seeprom_length,
+			.seen_val	= &seeprom_length_seen,
+		},
+		{
+			.name		= "value",
+			.type		= CMDL_U8,
+			.wanted_val	= &seeprom_value,
+			.seen_val	= &seeprom_value_seen,
+		},
 	};
 	int err;
 	struct ethtool_drvinfo drvinfo;
@@ -4553,11 +4731,27 @@ static int do_getmodule(struct cmd_context *ctx)
 	int err;
 
 	struct cmdline_info cmdline_geeprom[] = {
-		{ "offset", CMDL_U32, &geeprom_offset, NULL },
-		{ "length", CMDL_U32, &geeprom_length, NULL,
-		  0, &geeprom_length_seen },
-		{ "raw", CMDL_BOOL, &geeprom_dump_raw, NULL },
-		{ "hex", CMDL_BOOL, &geeprom_dump_hex, NULL },
+		{
+			.name		= "offset",
+			.type		= CMDL_U32,
+			.wanted_val	= &geeprom_offset,
+		},
+		{
+			.name		= "length",
+			.type		= CMDL_U32,
+			.wanted_val	= &geeprom_length,
+			.seen_val	= &geeprom_length_seen,
+		},
+		{
+			.name		= "raw",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &geeprom_dump_raw,
+		},
+		{
+			.name		= "hex",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &geeprom_dump_hex,
+		},
 	};
 
 	parse_generic_cmdline(ctx, &geeprom_changed,
@@ -4669,10 +4863,30 @@ static int do_seee(struct cmd_context *ctx)
 	int change = -1, change2 = 0;
 	struct ethtool_eee eeecmd;
 	struct cmdline_info cmdline_eee[] = {
-		{ "advertise",    CMDL_U32,  &adv_c,       &eeecmd.advertised },
-		{ "tx-lpi",       CMDL_BOOL, &lpi_c,   &eeecmd.tx_lpi_enabled },
-		{ "tx-timer",	  CMDL_U32,  &lpi_time_c, &eeecmd.tx_lpi_timer},
-		{ "eee",	  CMDL_BOOL, &eee_c,	   &eeecmd.eee_enabled},
+		{
+			.name		= "advertise",
+			.type		= CMDL_U32,
+			.wanted_val	= &adv_c,
+			.ioctl_val	= &eeecmd.advertised,
+		},
+		{
+			.name		= "tx-lpi",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &lpi_c,
+			.ioctl_val	= &eeecmd.tx_lpi_enabled,
+		},
+		{
+			.name		= "tx-timer",
+			.type		= CMDL_U32,
+			.wanted_val	= &lpi_time_c,
+			.ioctl_val	= &eeecmd.tx_lpi_timer,
+		},
+		{
+			.name		= "eee",
+			.type		= CMDL_BOOL,
+			.wanted_val	= &eee_c,
+			.ioctl_val	= &eeecmd.eee_enabled,
+		},
 	};
 
 	if (ctx->argc == 0)
-- 
2.28.0

[PATCH ethtool 7/7] build: add -Wextra to default CFLAGS

[Michal Kubecek]

As a result of previous commits, ethtool source now builds with gcc
versions 7-11 without any compiler warning with "-Wall -Wextra". Add
"-Wextra" to default cflags to make sure that any new warnings are
caught as early as possible.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 2abb2742c335..099182e8d6ad 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,4 @@
-AM_CFLAGS = -Wall
+AM_CFLAGS = -Wall -Wextra
 AM_CPPFLAGS = -I$(top_srcdir)/uapi
 LDADD = -lm
 
-- 
2.28.0

Re: [PATCH ethtool 1/7] netlink: get rid of signed/unsigned comparison warnings

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:19PM +0200, Michal Kubecek wrote:
> Get rid of compiler warnings about comparison between signed and
> unsigned integer values in netlink code.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
>  netlink/features.c | 4 ++--
>  netlink/netlink.c  | 4 ++--
>  netlink/netlink.h  | 2 +-
>  netlink/nlsock.c   | 2 +-
>  netlink/parser.c   | 2 +-
>  netlink/settings.c | 6 +++---
>  6 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/netlink/features.c b/netlink/features.c
> index 8b5b8588ca23..f5862e97a265 100644
> --- a/netlink/features.c
> +++ b/netlink/features.c
> @@ -149,7 +149,7 @@ int dump_features(const struct nlattr *const *tb,
>  			continue;
>  
>  		for (j = 0; j < results.count; j++) {
> -			if (feature_flags[j] == i) {
> +			if (feature_flags[j] == (int)i) {
>  				n_match++;
>  				flag_value = flag_value ||
>  					feature_on(results.active, j);
> @@ -163,7 +163,7 @@ int dump_features(const struct nlattr *const *tb,
>  		for (j = 0; j < results.count; j++) {
>  			const char *name = get_string(feature_names, j);
>  
> -			if (feature_flags[j] != i)
> +			if (feature_flags[j] != (int)i)

Hi Michal

Would it be better to make feature_flags an unsigned int * ? And
change the -1 to MAX_UNIT?

       Andrew

Re: [PATCH ethtool 2/7] ioctl: check presence of eeprom length argument properly

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:22PM +0200, Michal Kubecek wrote:
> In do_geeprom(), do_seprom() and do_getmodule(), check if user used
> "length" command line argument is done by setting the value to -1 before
> parsing and checking if it changed. This is quite ugly and also causes
> compiler warnings as the variable is u32.
> 
> Use proper "seen" flag to let parser tell us if the argument was used.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH ethtool 3/7] ioctl: get rid of signed/unsigned comparison warnings

[Andrew Lunn]

> -	while (arg_num < ctx->argc) {
> +	while (arg_num < (unsigned int)ctx->argc) {

Did you try changing ctx->argc to an unsigned int? I guess there would
be less casts that way, and it is a more logical type for this.

    Andrew

Re: [PATCH ethtool 4/7] get rid of signed/unsigned comparison warnings in register dump parsers

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:29PM +0200, Michal Kubecek wrote:
> All of these are avoided by declaring a variable (mostly loop iterators)
> holding only unsigned values as unsigned.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH ethtool 5/7] settings: simplify link_mode_info[] initializers

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:32PM +0200, Michal Kubecek wrote:
> Use macro helpers to make link_mode_info[] initializers easier to read and
> less prone to mistakes. As a bonus, this gets rid of "missing field
> initializer" warnings in netlink/settings.c
> 
> This commit should have no effect on resulting code (checked with gcc-11
> and -O2).
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH ethtool 6/7] ioctl: convert cmdline_info arrays to named initializers

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:35PM +0200, Michal Kubecek wrote:
> To get rid of remaining "missing field initializer" compiler warnings,
> convert arrays of struct cmdline_info used for command line parser to
> named initializers. This also makes the initializers easier to read.
> 
> This commit should have no effect on resulting code (checked with gcc-11
> and -O2).
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH ethtool 7/7] build: add -Wextra to default CFLAGS

[Andrew Lunn]

On Sun, Aug 09, 2020 at 11:24:38PM +0200, Michal Kubecek wrote:
> As a result of previous commits, ethtool source now builds with gcc
> versions 7-11 without any compiler warning with "-Wall -Wextra". Add
> "-Wextra" to default cflags to make sure that any new warnings are
> caught as early as possible.
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

RE: [PATCH ethtool 3/7] ioctl: get rid of signed/unsigned comparison warnings

[David Laight]

From: Andrew Lunn
> > -	while (arg_num < ctx->argc) {
> > +	while (arg_num < (unsigned int)ctx->argc) {
> 
> Did you try changing ctx->argc to an unsigned int? I guess there would
> be less casts that way, and it is a more logical type for this.

My favourite solution is to use '+ 0u' to force the signed
integer to unsigned.
Less likely to hide another bug than the cast.

But changing the type is better.
I just wish they'd fix gcc so that it didn't complain
if you'd just done a test that excluded negative values.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH ethtool 1/7] netlink: get rid of signed/unsigned comparison warnings

[Michal Kubecek]

On Mon, Aug 10, 2020 at 04:11:22PM +0200, Andrew Lunn wrote:
> On Sun, Aug 09, 2020 at 11:24:19PM +0200, Michal Kubecek wrote:
> > Get rid of compiler warnings about comparison between signed and
> > unsigned integer values in netlink code.
> > 
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> > ---
> >  netlink/features.c | 4 ++--
> >  netlink/netlink.c  | 4 ++--
> >  netlink/netlink.h  | 2 +-
> >  netlink/nlsock.c   | 2 +-
> >  netlink/parser.c   | 2 +-
> >  netlink/settings.c | 6 +++---
> >  6 files changed, 10 insertions(+), 10 deletions(-)
> > 
> > diff --git a/netlink/features.c b/netlink/features.c
> > index 8b5b8588ca23..f5862e97a265 100644
> > --- a/netlink/features.c
> > +++ b/netlink/features.c
> > @@ -149,7 +149,7 @@ int dump_features(const struct nlattr *const *tb,
> >  			continue;
> >  
> >  		for (j = 0; j < results.count; j++) {
> > -			if (feature_flags[j] == i) {
> > +			if (feature_flags[j] == (int)i) {
> >  				n_match++;
> >  				flag_value = flag_value ||
> >  					feature_on(results.active, j);
> > @@ -163,7 +163,7 @@ int dump_features(const struct nlattr *const *tb,
> >  		for (j = 0; j < results.count; j++) {
> >  			const char *name = get_string(feature_names, j);
> >  
> > -			if (feature_flags[j] != i)
> > +			if (feature_flags[j] != (int)i)
> 
> Hi Michal
> 
> Would it be better to make feature_flags an unsigned int * ? And
> change the -1 to MAX_UNIT?

It certainly would. I was actually thinking about this solution for
a moment but then I managed to mistake feature_flags with off_flag_def
and convinced myself that it's shared with ioctl code so that changing
its type would require changes there as well. Thank you for pointing
this out.

Michal

Re: [PATCH ethtool 3/7] ioctl: get rid of signed/unsigned comparison warnings

[Michal Kubecek]

On Mon, Aug 10, 2020 at 04:19:24PM +0200, Andrew Lunn wrote:
> > -	while (arg_num < ctx->argc) {
> > +	while (arg_num < (unsigned int)ctx->argc) {
> 
> Did you try changing ctx->argc to an unsigned int? I guess there would
> be less casts that way, and it is a more logical type for this.
> 
>     Andrew

I tried now and the number of changes in ethtool.c is not as bad as
I thought. I even found one missing check which could allow argc to fall
below 0.

Michal