git: 982cd5ae8ef6 - main - ifconfig: split argument parsing and actual execution logic
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 10 May 2023 10:42:41 UTC
The branch main has been updated by melifaro:
URL: https://cgit.FreeBSD.org/src/commit/?id=982cd5ae8ef6541fffcb4251a5a2ab59d706df10
commit 982cd5ae8ef6541fffcb4251a5a2ab59d706df10
Author: Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2023-05-10 09:58:56 +0000
Commit: Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2023-05-10 10:39:12 +0000
ifconfig: split argument parsing and actual execution logic
Reduce the amount of global variables by creating the dedicated
ifconfig_args structure and use it as a context-passing variable.
Simplify the code by moving all argument preparation code a
separate function.
Reviewed by: kp (previous version)
Differential Revision: https://reviews.freebsd.org/D39932
MFC after: 2 weeks
---
sbin/ifconfig/ifconfig.c | 148 ++++++++++++++++++++++++++---------------------
sbin/ifconfig/ifconfig.h | 21 ++++++-
sbin/ifconfig/ifmedia.c | 2 +-
3 files changed, 103 insertions(+), 68 deletions(-)
diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c
index 462d543125c4..88c720223c32 100644
--- a/sbin/ifconfig/ifconfig.c
+++ b/sbin/ifconfig/ifconfig.c
@@ -98,10 +98,10 @@ int doalias;
int clearaddr;
int newaddr = 1;
int verbose;
-int noload;
int printifname = 0;
-int supmedia = 0;
+struct ifconfig_args args;
+
int printkeys = 0; /* Print keying material for interfaces. */
int exit_code = 0;
@@ -112,7 +112,7 @@ static bool group_member(const char *ifname, const char *match,
const char *nomatch);
static int ifconfig(int argc, char *const *argv, int iscreate,
const struct afswtch *afp);
-static void status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
+static void status(struct ifconfig_args *args, const struct sockaddr_dl *sdl,
struct ifaddrs *ifa);
static void tunnel_status(int s);
static _Noreturn void usage(void);
@@ -403,26 +403,21 @@ void printifnamemaybe()
printf("%s\n", name);
}
+static void list_interfaces(struct ifconfig_args *args);
+
int
main(int argc, char *argv[])
{
- int c, all, namesonly, downonly, uponly;
+ int c;
const struct afswtch *afp = NULL;
int ifindex;
- struct ifaddrs *ifap, *sifap, *ifa;
- struct ifreq paifr;
- const struct sockaddr_dl *sdl;
- char options[1024], *cp, *envformat, *namecp = NULL;
- struct ifa_queue q = TAILQ_HEAD_INITIALIZER(q);
- struct ifa_order_elt *cur, *tmp;
- const char *ifname, *matchgroup, *nogroup;
+ char options[1024], *envformat;
+ const char *ifname;
struct option *p;
size_t iflen;
int flags;
- all = downonly = uponly = namesonly = noload = verbose = 0;
f_inet = f_inet6 = f_ether = f_addr = NULL;
- matchgroup = nogroup = NULL;
lifh = ifconfig_open();
if (lifh == NULL)
@@ -445,10 +440,10 @@ main(int argc, char *argv[])
while ((c = getopt(argc, argv, options)) != -1) {
switch (c) {
case 'a': /* scan all interfaces */
- all++;
+ args.all = true;
break;
case 'd': /* restrict scan to "down" interfaces */
- downonly++;
+ args.downonly = true;
break;
case 'f':
if (optarg == NULL)
@@ -456,33 +451,33 @@ main(int argc, char *argv[])
setformat(optarg);
break;
case 'G':
- if (optarg == NULL || all == 0)
+ if (optarg == NULL || args.all == 0)
usage();
- nogroup = optarg;
+ args.nogroup = optarg;
break;
case 'k':
- printkeys++;
+ args.printkeys = true;
break;
case 'l': /* scan interface names only */
- namesonly++;
+ args.namesonly++;
break;
case 'm': /* show media choices in status */
- supmedia = 1;
+ args.supmedia = true;
break;
case 'n': /* suppress module loading */
- noload++;
+ args.noload = true;
break;
case 'u': /* restrict scan to "up" interfaces */
- uponly++;
+ args.uponly = true;
break;
case 'v':
- verbose++;
+ args.verbose++;
break;
case 'g':
- if (all) {
+ if (args.all) {
if (optarg == NULL)
usage();
- matchgroup = optarg;
+ args.matchgroup = optarg;
break;
}
/* FALLTHROUGH */
@@ -500,20 +495,24 @@ 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 (namesonly && (all || supmedia))
+ if (args.namesonly && (args.all || args.supmedia))
usage();
/* nonsense.. */
- if (uponly && downonly)
+ if (args.uponly && args.downonly)
usage();
/* no arguments is equivalent to '-a' */
- if (!namesonly && argc < 1)
- all = 1;
+ if (!args.namesonly && argc < 1)
+ args.all = 1;
/* -a and -l allow an address family arg to limit the output */
- if (all || namesonly) {
+ if (args.all || args.namesonly) {
if (argc > 1)
usage();
@@ -538,7 +537,7 @@ main(int argc, char *argv[])
argc--, argv++;
/* check and maybe load support for this interface */
- ifmaybeload(ifname);
+ ifmaybeload(&args, ifname);
ifindex = if_nametoindex(ifname);
if (ifindex == 0) {
@@ -606,17 +605,38 @@ main(int argc, char *argv[])
} else {
flags = getifflags(name, -1, false);
if (!(((flags & IFF_CANTCONFIG) != 0) ||
- (downonly && (flags & IFF_UP) != 0) ||
- (uponly && (flags & IFF_UP) == 0)))
+ (args.downonly && (flags & IFF_UP) != 0) ||
+ (args.uponly && (flags & IFF_UP) == 0)))
ifconfig(argc, argv, 0, afp);
}
goto done;
}
+ args.argc = argc;
+ args.argv = argv;
+
+ list_interfaces(&args);
+
+done:
+ freeformat();
+ ifconfig_close(lifh);
+ exit(exit_code);
+}
+
+static void
+list_interfaces(struct ifconfig_args *args)
+{
+ struct ifa_queue q = TAILQ_HEAD_INITIALIZER(q);
+ struct ifaddrs *ifap, *sifap, *ifa;
+ struct ifa_order_elt *cur, *tmp;
+ char *namecp = NULL;
+ int ifindex;
+ size_t iflen;
+
if (getifaddrs(&ifap) != 0)
err(EXIT_FAILURE, "getifaddrs");
- cp = NULL;
+ char *cp = NULL;
if (calcorders(ifap, &q) != 0)
err(EXIT_FAILURE, "calcorders");
@@ -628,7 +648,10 @@ main(int argc, char *argv[])
ifindex = 0;
for (ifa = sifap; ifa; ifa = ifa->ifa_next) {
- memset(&paifr, 0, sizeof(paifr));
+ struct ifreq paifr = {};
+ const struct sockaddr_dl *sdl;
+ const char *ifname;
+
strlcpy(paifr.ifr_name, ifa->ifa_name, sizeof(paifr.ifr_name));
if (sizeof(paifr.ifr_addr) >= ifa->ifa_addr->sa_len) {
memcpy(&paifr.ifr_addr, ifa->ifa_addr,
@@ -641,7 +664,7 @@ main(int argc, char *argv[])
sdl = (const struct sockaddr_dl *) ifa->ifa_addr;
else
sdl = NULL;
- if (cp != NULL && strcmp(cp, ifa->ifa_name) == 0 && !namesonly)
+ if (cp != NULL && strcmp(cp, ifa->ifa_name) == 0 && !args->namesonly)
continue;
iflen = strlcpy(name, ifa->ifa_name, sizeof(name));
if (iflen >= sizeof(name)) {
@@ -653,21 +676,21 @@ main(int argc, char *argv[])
if ((ifa->ifa_flags & IFF_CANTCONFIG) != 0)
continue;
- if (downonly && (ifa->ifa_flags & IFF_UP) != 0)
+ if (args->downonly && (ifa->ifa_flags & IFF_UP) != 0)
continue;
- if (uponly && (ifa->ifa_flags & IFF_UP) == 0)
+ if (args->uponly && (ifa->ifa_flags & IFF_UP) == 0)
continue;
- if (!group_member(ifa->ifa_name, matchgroup, nogroup))
+ if (!group_member(ifa->ifa_name, args->matchgroup, args->nogroup))
continue;
/*
* Are we just listing the interfaces?
*/
- if (namesonly) {
+ if (args->namesonly) {
if (namecp == cp)
continue;
- if (afp != NULL) {
+ if (args->afp != NULL) {
/* special case for "ether" address family */
- if (!strcmp(afp->af_name, "ether")) {
+ if (!strcmp(args->afp->af_name, "ether")) {
if (sdl == NULL ||
(sdl->sdl_type != IFT_ETHER &&
sdl->sdl_type != IFT_L2VLAN &&
@@ -676,7 +699,7 @@ main(int argc, char *argv[])
continue;
} else {
if (ifa->ifa_addr->sa_family
- != afp->af_af)
+ != args->afp->af_af)
continue;
}
}
@@ -689,19 +712,14 @@ main(int argc, char *argv[])
}
ifindex++;
- if (argc > 0)
- ifconfig(argc, argv, 0, afp);
+ if (args->argc > 0)
+ ifconfig(args->argc, args->argv, 0, args->afp);
else
- status(afp, sdl, ifa);
+ status(args, sdl, ifa);
}
- if (namesonly)
+ if (args->namesonly)
printf("\n");
freeifaddrs(ifap);
-
-done:
- freeformat();
- ifconfig_close(lifh);
- exit(exit_code);
}
/*
@@ -1421,7 +1439,7 @@ unsetifdescr(const char *val, int value, int s, const struct afswtch *afp)
* specified, show only it; otherwise, show them all.
*/
static void
-status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
+status(struct ifconfig_args *args, const struct sockaddr_dl *sdl,
struct ifaddrs *ifa)
{
struct ifaddrs *ift;
@@ -1432,13 +1450,13 @@ status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
int allfamilies, s, type;
bool first, val;
- if (afp == NULL) {
+ if (args->afp == NULL) {
allfamilies = 1;
ifr.ifr_addr.sa_family = AF_LOCAL;
} else {
allfamilies = 0;
ifr.ifr_addr.sa_family =
- afp->af_af == AF_LINK ? AF_LOCAL : afp->af_af;
+ args->afp->af_af == AF_LINK ? AF_LOCAL : args->afp->af_af;
}
strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
@@ -1503,7 +1521,7 @@ status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
}
}
}
- if (supmedia) {
+ if (args->supmedia) {
printf("\tcapabilities");
cookie = NULL;
for (first = true;; first = false) {
@@ -1526,7 +1544,7 @@ status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
} else if (ifr.ifr_curcap != 0) {
printb("\toptions", ifr.ifr_curcap, IFCAPBITS);
putchar('\n');
- if (supmedia && ifr.ifr_reqcap != 0) {
+ if (args->supmedia && ifr.ifr_reqcap != 0) {
printb("\tcapabilities", ifr.ifr_reqcap,
IFCAPBITS);
putchar('\n');
@@ -1546,8 +1564,8 @@ status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
p = af_getbyfamily(ift->ifa_addr->sa_family);
if (p != NULL && p->af_status != NULL)
p->af_status(s, ift);
- } else if (afp->af_af == ift->ifa_addr->sa_family)
- afp->af_status(s, ift);
+ } else if (args->afp->af_af == ift->ifa_addr->sa_family)
+ args->afp->af_status(s, ift);
}
#if 0
if (allfamilies || afp->af_af == AF_LINK) {
@@ -1568,15 +1586,15 @@ status(const struct afswtch *afp, const struct sockaddr_dl *sdl,
#endif
if (allfamilies)
af_other_status(s);
- else if (afp->af_other_status != NULL)
- afp->af_other_status(s);
+ else if (args->afp->af_other_status != NULL)
+ args->afp->af_other_status(s);
strlcpy(ifs.ifs_name, name, sizeof ifs.ifs_name);
if (ioctl(s, SIOCGIFSTATUS, &ifs) == 0)
printf("%s", ifs.ascii);
- if (verbose > 0)
- sfp_status(s, &ifr, verbose);
+ if (args->verbose > 0)
+ sfp_status(s, &ifr, args->verbose);
close(s);
return;
@@ -1653,7 +1671,7 @@ print_vhid(const struct ifaddrs *ifa, const char *s)
}
void
-ifmaybeload(const char *name)
+ifmaybeload(struct ifconfig_args *args, const char *name)
{
#define MOD_PREFIX_LEN 3 /* "if_" */
struct module_stat mstat;
@@ -1664,7 +1682,7 @@ ifmaybeload(const char *name)
bool found;
/* loading suppressed by the user */
- if (noload)
+ if (args->noload)
return;
/* trim the interface number off the end */
diff --git a/sbin/ifconfig/ifconfig.h b/sbin/ifconfig/ifconfig.h
index 26f68d67cec2..e1cd8a628f9a 100644
--- a/sbin/ifconfig/ifconfig.h
+++ b/sbin/ifconfig/ifconfig.h
@@ -39,6 +39,7 @@
#pragma once
#include <libifconfig.h>
+#include <stdbool.h>
#define __constructor __attribute__((constructor))
@@ -182,6 +183,22 @@ struct afswtch {
};
void af_register(struct afswtch *);
+struct ifconfig_args {
+ bool all; /* Match everything */
+ bool downonly; /* Down-only items */
+ bool uponly; /* Up-only items */
+ bool namesonly; /* Output only names */
+ bool noload; /* Do not load relevant kernel modules */
+ bool supmedia; /* Supported media */
+ bool printkeys; /* Print security keys */
+ int verbose; /* verbosity level */
+ int argc;
+ char **argv;
+ const char *matchgroup; /* Group name to match */
+ const char *nogroup; /* Group name to exclude */
+ const struct afswtch *afp; /* AF we're operating on */
+};
+
struct option {
const char *opt;
const char *opt_usage;
@@ -194,12 +211,12 @@ extern ifconfig_handle_t *lifh;
extern struct ifreq ifr;
extern char name[IFNAMSIZ]; /* name of interface */
extern int allmedia;
-extern int supmedia;
extern int printkeys;
extern int newaddr;
extern int verbose;
extern int printifname;
extern int exit_code;
+extern struct ifconfig_args args;
void setifcap(const char *, int value, int s, const struct afswtch *);
void setifcapnv(const char *vname, const char *arg, int s,
@@ -208,7 +225,7 @@ void setifcapnv(const char *vname, const char *arg, int s,
void Perror(const char *cmd);
void printb(const char *s, unsigned value, const char *bits);
-void ifmaybeload(const char *name);
+void ifmaybeload(struct ifconfig_args *args, const char *name);
typedef int clone_match_func(const char *);
typedef void clone_callback_func(int, struct ifreq *);
diff --git a/sbin/ifconfig/ifmedia.c b/sbin/ifconfig/ifmedia.c
index aacf34a13248..3014d19b00ff 100644
--- a/sbin/ifconfig/ifmedia.c
+++ b/sbin/ifconfig/ifmedia.c
@@ -144,7 +144,7 @@ media_status(int s)
putchar('\n');
}
- if (supmedia) {
+ if (args.supmedia) {
printf("\tsupported media:\n");
for (size_t i = 0; i < ifmr->ifm_count; ++i) {
printf("\t\t");