netdev - [PATCH ethtool 2/3] ethtool: use dummy args[] entry for no-args case

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: <20210813171938.1127891-3-kuba@kernel.org>
Date:   Fri, 13 Aug 2021 10:19:37 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     mkubecek@...e.cz, andrew@...n.ch
Cc:     netdev@...r.kernel.org, dcavalca@...com, filbranden@...com,
        michel@...com, Jakub Kicinski <kuba@...nel.org>
Subject: [PATCH ethtool 2/3] ethtool: use dummy args[] entry for no-args case

Simplify the code flow further by adding a struct option
entry for the no-args case (e.g. "ethtool eth0").

This leads to a slight change in the help output, there
will now be an extra space between FLAGS and DEVICE in
that case:

  ethtool [ FLAGS ]  DEVNAME	Display standard information about device

but hopefully that's okay.

Note that this patch adds a false-positive warning with GCC 11:

ethtool.c: In function ‘find_option’:
ethtool.c:6082:29: warning: offset ‘1’ outside bounds of constant string [-Warray-bounds]
 6082 |                         opt += len + 1;
      |                         ~~~~^~~~~~~~~~

we'll never get to that code if the string is empty.

Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
 ethtool.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 8cf1b13e4176..9e02fe4f09a5 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5627,6 +5627,13 @@ struct option {
 };
 
 static const struct option args[] = {
+	{
+		/* "default" entry when no switch is used */
+		.opts	= "",
+		.func	= do_gset,
+		.nlfunc	= nl_gset,
+		.help	= "Display standard information about device",
+	},
 	{
 		.opts	= "-s|--change",
 		.func	= do_sset,
@@ -6041,10 +6048,7 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
 
 	/* ethtool -h */
 	fprintf(stdout, PACKAGE " version " VERSION "\n");
-	fprintf(stdout,
-		"Usage:\n"
-		"        ethtool [ FLAGS ] DEVNAME\t"
-		"Display standard information about device\n");
+	fprintf(stdout,	"Usage:\n");
 	for (i = 0; args[i].opts; i++) {
 		fputs("        ethtool [ FLAGS ] ", stdout);
 		fprintf(stdout, "%s %s\t%s\n",
@@ -6287,11 +6291,7 @@ static int ioctl_init(struct cmd_context *ctx, bool no_dev)
 
 int main(int argc, char **argp)
 {
-	int (*func)(struct cmd_context *);
 	struct cmd_context ctx = {};
-	nl_func_t nlfunc = NULL;
-	nl_chk_t nlchk = NULL;
-	bool no_dev;
 	int ret;
 	int k;
 
@@ -6345,22 +6345,16 @@ int main(int argc, char **argp)
 		exit_bad_args();
 
 	k = find_option(*argp);
-	if (k >= 0) {
+	if (k > 0) {
 		argp++;
 		argc--;
-		func = args[k].func;
-		nlfunc = args[k].nlfunc;
-		nlchk = args[k].nlchk;
-		no_dev = args[k].no_dev;
 	} else {
 		if ((*argp)[0] == '-')
 			exit_bad_args();
-		nlfunc = nl_gset;
-		func = do_gset;
-		no_dev = false;
+		k = 0;
 	}
 
-	if (!no_dev) {
+	if (!args[k].no_dev) {
 		ctx.devname = *argp++;
 		argc--;
 
@@ -6369,11 +6363,11 @@ int main(int argc, char **argp)
 	}
 	ctx.argc = argc;
 	ctx.argp = argp;
-	netlink_run_handler(&ctx, nlchk, nlfunc, !func);
+	netlink_run_handler(&ctx, args[k].nlchk, args[k].nlfunc, !args[k].func);
 
-	ret = ioctl_init(&ctx, no_dev);
+	ret = ioctl_init(&ctx, args[k].no_dev);
 	if (ret)
 		return ret;
 
-	return func(&ctx);
+	return args[k].func(&ctx);
 }
-- 
2.31.1