git: 197bff385e2c - main - ifconfig: split argument parsing

From: Alexander V. Chernikov <melifaro_at_FreeBSD.org>
Date: Sun, 21 May 2023 09:43:00 UTC
The branch main has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=197bff385e2caf82c52036f572e1cc79efc217f8

commit 197bff385e2caf82c52036f572e1cc79efc217f8
Author:     Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2023-05-20 11:14:39 +0000
Commit:     Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2023-05-21 09:42:11 +0000

    ifconfig: split argument parsing
    
    Simplify main() by factoring out argument parsing code.
    
    Reviewed by:    kp
    MFC after:      2 weeks
    Differential Revision: https://reviews.freebsd.org/D40179
---
 sbin/ifconfig/ifconfig.c | 165 ++++++++++++++++++++++++++++-------------------
 1 file changed, 98 insertions(+), 67 deletions(-)

diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c
index 8c3a7b4c0007..16ab651261d1 100644
--- a/sbin/ifconfig/ifconfig.c
+++ b/sbin/ifconfig/ifconfig.c
@@ -407,32 +407,34 @@ list_interfaces(struct ifconfig_args *args)
 #endif
 }
 
-int
-main(int argc, char *argv[])
+static char *
+args_peek(struct ifconfig_args *args)
 {
-	int c;
-	const struct afswtch *afp = NULL;
-	int ifindex;
-	char options[1024], *envformat;
-	struct option *p;
-	size_t iflen;
-	int flags;
+	if (args->argc > 0)
+		return (args->argv[0]);
+	return (NULL);
+}
 
-	f_inet = f_inet6 = f_ether = f_addr = NULL;
+static char *
+args_pop(struct ifconfig_args *args)
+{
+	if (args->argc == 0)
+		return (NULL);
 
-	lifh = ifconfig_open();
-	if (lifh == NULL)
-		err(EXIT_FAILURE, "ifconfig_open");
+	char *arg = args->argv[0];
 
-	envformat = getenv("IFCONFIG_FORMAT");
-	if (envformat != NULL)
-		setformat(envformat);
+	args->argc--;
+	args->argv++;
 
-	/*
-	 * Ensure we print interface name when expected to,
-	 * even if we terminate early due to error.
-	 */
-	atexit(printifnamemaybe);
+	return (arg);
+}
+
+static void
+args_parse(struct ifconfig_args *args, int argc, char *argv[])
+{
+	char options[1024];
+	struct option *p;
+	int c;
 
 	/* Parse leading line options */
 	strlcpy(options, "G:adf:klmnuv", sizeof(options));
@@ -441,10 +443,10 @@ main(int argc, char *argv[])
 	while ((c = getopt(argc, argv, options)) != -1) {
 		switch (c) {
 		case 'a':	/* scan all interfaces */
-			args.all = true;
+			args->all = true;
 			break;
 		case 'd':	/* restrict scan to "down" interfaces */
-			args.downonly = true;
+			args->downonly = true;
 			break;
 		case 'f':
 			if (optarg == NULL)
@@ -452,33 +454,33 @@ main(int argc, char *argv[])
 			setformat(optarg);
 			break;
 		case 'G':
-			if (optarg == NULL || args.all == 0)
+			if (optarg == NULL || args->all == 0)
 				usage();
-			args.nogroup = optarg;
+			args->nogroup = optarg;
 			break;
 		case 'k':
-			args.printkeys = true;
+			args->printkeys = true;
 			break;
 		case 'l':	/* scan interface names only */
-			args.namesonly++;
+			args->namesonly++;
 			break;
 		case 'm':	/* show media choices in status */
-			args.supmedia = true;
+			args->supmedia = true;
 			break;
 		case 'n':	/* suppress module loading */
-			args.noload = true;
+			args->noload = true;
 			break;
 		case 'u':	/* restrict scan to "up" interfaces */
-			args.uponly = true;
+			args->uponly = true;
 			break;
 		case 'v':
-			args.verbose++;
+			args->verbose++;
 			break;
 		case 'g':
-			if (args.all) {
+			if (args->all) {
 				if (optarg == NULL)
 					usage();
-				args.matchgroup = optarg;
+				args->matchgroup = optarg;
 				break;
 			}
 			/* FALLTHROUGH */
@@ -496,30 +498,26 @@ main(int argc, char *argv[])
 	argc -= optind;
 	argv += optind;
 
-	/* Sync global variables */
-	printkeys = args.printkeys;
-	verbose = args.verbose;
-
 	/* -l cannot be used with -a or -m */
-	if (args.namesonly && (args.all || args.supmedia))
+	if (args->namesonly && (args->all || args->supmedia))
 		usage();
 
 	/* nonsense.. */
-	if (args.uponly && args.downonly)
+	if (args->uponly && args->downonly)
 		usage();
 
 	/* no arguments is equivalent to '-a' */
-	if (!args.namesonly && argc < 1)
-		args.all = 1;
+	if (!args->namesonly && argc < 1)
+		args->all = 1;
 
 	/* -a and -l allow an address family arg to limit the output */
-	if (args.all || args.namesonly) {
+	if (args->all || args->namesonly) {
 		if (argc > 1)
 			usage();
 
-		ifindex = 0;
 		if (argc == 1) {
-			afp = af_getbyname(*argv);
+			const struct afswtch *afp = af_getbyname(*argv);
+
 			if (afp == NULL) {
 				warnx("Address family '%s' unknown.", *argv);
 				usage();
@@ -527,32 +525,68 @@ main(int argc, char *argv[])
 			if (afp->af_name != NULL)
 				argc--, argv++;
 			/* leave with afp non-zero */
+			args->afp = afp;
 		}
 	} else {
 		/* not listing, need an argument */
 		if (argc < 1)
 			usage();
+	}
 
-		args.ifname = *argv;
-		argc--, argv++;
+	args->argc = argc;
+	args->argv = argv;
+
+	/* Sync global variables */
+	printkeys = args->printkeys;
+	verbose = args->verbose;
+}
+
+int
+main(int ac, char *av[])
+{
+	char *envformat;
+	size_t iflen;
+	int flags;
+
+	f_inet = f_inet6 = f_ether = f_addr = NULL;
+
+	lifh = ifconfig_open();
+	if (lifh == NULL)
+		err(EXIT_FAILURE, "ifconfig_open");
+
+	envformat = getenv("IFCONFIG_FORMAT");
+	if (envformat != NULL)
+		setformat(envformat);
+
+	/*
+	 * Ensure we print interface name when expected to,
+	 * even if we terminate early due to error.
+	 */
+	atexit(printifnamemaybe);
+
+	args_parse(&args, ac, av);
+
+	if (!args.all && !args.namesonly) {
+		/* not listing, need an argument */
+		args.ifname = args_pop(&args);
 
 		/* check and maybe load support for this interface */
 		ifmaybeload(&args, args.ifname);
 
-		ifindex = if_nametoindex(args.ifname);
-		if (ifindex == 0) {
+		char *arg = args_peek(&args);
+		if (if_nametoindex(args.ifname) == 0) {
 			/*
 			 * NOTE:  We must special-case the `create' command
 			 * right here as we would otherwise fail when trying
 			 * to find the interface.
 			 */
-			if (argc > 0 && (strcmp(argv[0], "create") == 0 ||
-			    strcmp(argv[0], "plumb") == 0)) {
+			if (arg != NULL && (strcmp(arg, "create") == 0 ||
+			    strcmp(arg, "plumb") == 0)) {
 				iflen = strlcpy(name, args.ifname, sizeof(name));
 				if (iflen >= sizeof(name))
 					errx(1, "%s: cloning name too long",
 					    args.ifname);
-				ifconfig(argc, argv, 1, NULL);
+				ifconfig(args.argc, args.argv, 1, NULL);
 				exit(exit_code);
 			}
 #ifdef JAIL
@@ -561,12 +595,12 @@ main(int argc, char *argv[])
 			 * right here as we would otherwise fail when trying
 			 * to find the interface as it lives in another vnet.
 			 */
-			if (argc > 0 && (strcmp(argv[0], "-vnet") == 0)) {
+			if (arg != NULL && (strcmp(arg, "-vnet") == 0)) {
 				iflen = strlcpy(name, args.ifname, sizeof(name));
 				if (iflen >= sizeof(name))
 					errx(1, "%s: interface name too long",
 					    args.ifname);
-				ifconfig(argc, argv, 0, NULL);
+				ifconfig(args.argc, args.argv, 0, NULL);
 				exit(exit_code);
 			}
 #endif
@@ -576,21 +610,21 @@ main(int argc, char *argv[])
 			 * Do not allow use `create` command as hostname if
 			 * address family is not specified.
 			 */
-			if (argc > 0 && (strcmp(argv[0], "create") == 0 ||
-			    strcmp(argv[0], "plumb") == 0)) {
-				if (argc == 1)
+			if (arg != NULL && (strcmp(arg, "create") == 0 ||
+			    strcmp(arg, "plumb") == 0)) {
+				if (args.argc == 1)
 					errx(1, "interface %s already exists",
 					    args.ifname);
-				argc--, argv++;
+				args_pop(&args);
 			}
 		}
 	}
 
 	/* Check for address family */
-	if (argc > 0) {
-		afp = af_getbyname(*argv);
-		if (afp != NULL)
-			argc--, argv++;
+	if (args.argc > 0) {
+		args.afp = af_getbyname(args_peek(&args));
+		if (args.afp != NULL)
+			args_pop(&args);
 	}
 
 	/*
@@ -598,7 +632,7 @@ main(int argc, char *argv[])
 	 * which doesn't require building, sorting, and searching the entire
 	 * system address list
 	 */
-	if ((argc > 0) && (args.ifname != NULL)) {
+	if ((args.argc > 0) && (args.ifname != NULL)) {
 		iflen = strlcpy(name, args.ifname, sizeof(name));
 		if (iflen >= sizeof(name)) {
 			warnx("%s: interface name too long, skipping", args.ifname);
@@ -607,15 +641,12 @@ main(int argc, char *argv[])
 			if (!(((flags & IFF_CANTCONFIG) != 0) ||
 				(args.downonly && (flags & IFF_UP) != 0) ||
 				(args.uponly && (flags & IFF_UP) == 0)))
-				ifconfig(argc, argv, 0, afp);
+				ifconfig(args.argc, args.argv, 0, args.afp);
 		}
 		goto done;
 	}
 
-	args.afp = afp;
-	args.allfamilies = afp == NULL;
-	args.argc = argc;
-	args.argv = argv;
+	args.allfamilies = args.afp == NULL;
 
 	list_interfaces(&args);