Date: Wed, 10 May 2023 10:42:41 GMT From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 982cd5ae8ef6 - main - ifconfig: split argument parsing and actual execution logic Message-ID: <202305101042.34AAgf5O024479@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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");
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202305101042.34AAgf5O024479>