From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTP id DAA255A0082 for ; Wed, 8 Feb 2023 18:48:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675878520; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WHuPQkdchGxaENp7+DVXpqzpyKjTz513sv/5nTqGB4I=; b=MTdM4KTN50nsoVOf5KgO1V9Bn7i6gB88L7i0r7PmjqmAEaC7/oHzMDmRKey6mLqQ66kHvr xcAZbH7cRvEHoFA2EUOJpGGNiQNl8nhsQb7A19IuJ5B1kv9hP/0K2j2t36iNdRSEvXAxsy znnhmvFGveL22QqLkm+iM0ZWdEGUa+s= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-249-xI-GE5_rOB2W8MYmKDTbzQ-1; Wed, 08 Feb 2023 12:48:39 -0500 X-MC-Unique: xI-GE5_rOB2W8MYmKDTbzQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9A93C101AA5F; Wed, 8 Feb 2023 17:48:39 +0000 (UTC) Received: from vhost3.router.laine.org (unknown [10.2.17.81]) by smtp.corp.redhat.com (Postfix) with ESMTP id 79C662166B29; Wed, 8 Feb 2023 17:48:39 +0000 (UTC) From: Laine Stump To: passt-dev@passt.top, laine@laine.org Subject: [PATCH v2 3/9] eliminate most calls to usage() in conf() Date: Wed, 8 Feb 2023 12:48:32 -0500 Message-Id: <20230208174838.1680517-4-laine@redhat.com> In-Reply-To: <20230208174838.1680517-1-laine@redhat.com> References: <20230208174838.1680517-1-laine@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true Message-ID-Hash: WMFCDXKDUSRLBVBN5B4M7KMQQUNVRDEJ X-Message-ID-Hash: WMFCDXKDUSRLBVBN5B4M7KMQQUNVRDEJ X-MailFrom: laine@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.3 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Nearly all of the calls to usage() in conf() occur immediately after logging a more detailed error message, and the fact that these errors are occuring indicates that the user has already seen the passt usage message (or read the manpage). Spamming the logfile with the complete contents of the usage message serves only to obscure the more detailed error message. The only time when the full usage message should be output is if the user explicitly asks for it with -h (or its synonyms) AS a start to eliminating the excessive calls to usage(), this patch replaces most calls to err() followed by usage() with a call to errexit() instead. A few other usage() calls remain, but their removal involves bit more nuance that should be properly explained in separate commit messages. Signed-off-by: Laine Stump --- conf.c | 339 +++++++++++++++++++++------------------------------------ 1 file changed, 123 insertions(+), 216 deletions(-) diff --git a/conf.c b/conf.c index c37552d..e5035f7 100644 --- a/conf.c +++ b/conf.c @@ -1156,69 +1156,57 @@ void conf(struct ctx *c, int argc, char **argv) case 0: break; case 2: - if (c->mode != MODE_PASTA) { - err("--userns is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--userns is for pasta mode only"); ret = snprintf(userns, sizeof(userns), "%s", optarg); - if (ret <= 0 || ret >= (int)sizeof(userns)) { - err("Invalid userns: %s", optarg); - usage(argv[0]); - } + if (ret <= 0 || ret >= (int)sizeof(userns)) + errexit("Invalid userns: %s", optarg); + break; case 3: - if (c->mode != MODE_PASTA) { - err("--netns is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--netns is for pasta mode only"); ret = conf_netns_opt(netns, optarg); if (ret < 0) usage(argv[0]); break; case 4: - if (c->mode != MODE_PASTA) { - err("--ns-mac-addr is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--ns-mac-addr is for pasta mode only"); for (i = 0; i < ETH_ALEN; i++) { errno = 0; b = strtol(optarg + (intptr_t)i * 3, NULL, 16); - if (b < 0 || b > UCHAR_MAX || errno) { - err("Invalid MAC address: %s", optarg); - usage(argv[0]); - } + if (b < 0 || b > UCHAR_MAX || errno) + errexit("Invalid MAC address: %s", optarg); + c->mac_guest[i] = b; } break; case 5: - if (c->mode != MODE_PASTA) { - err("--dhcp-dns is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--dhcp-dns is for pasta mode only"); + c->no_dhcp_dns = 0; break; case 6: - if (c->mode != MODE_PASST) { - err("--no-dhcp-dns is for passt mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASST) + errexit("--no-dhcp-dns is for passt mode only"); + c->no_dhcp_dns = 1; break; case 7: - if (c->mode != MODE_PASTA) { - err("--dhcp-search is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--dhcp-search is for pasta mode only"); + c->no_dhcp_dns_search = 0; break; case 8: - if (c->mode != MODE_PASST) { - err("--no-dhcp-search is for passt mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASST) + errexit("--no-dhcp-search is for passt mode only"); + c->no_dhcp_dns_search = 1; break; case 9: @@ -1235,50 +1223,39 @@ void conf(struct ctx *c, int argc, char **argv) !IN4_IS_ADDR_LOOPBACK(&c->ip4.dns_match)) break; - err("Invalid DNS forwarding address: %s", optarg); - usage(argv[0]); + errexit("Invalid DNS forwarding address: %s", optarg); break; case 10: - if (c->mode != MODE_PASTA) { - err("--no-netns-quit is for pasta mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASTA) + errexit("--no-netns-quit is for pasta mode only"); + c->no_netns_quit = 1; break; case 11: - if (c->trace) { - err("Multiple --trace options given"); - usage(argv[0]); - } + if (c->trace) + errexit("Multiple --trace options given"); - if (c->quiet) { - err("Either --trace or --quiet"); - usage(argv[0]); - } + if (c->quiet) + errexit("Either --trace or --quiet"); c->trace = c->debug = 1; break; case 12: - if (runas) { - err("Multiple --runas options given"); - usage(argv[0]); - } + if (runas) + errexit("Multiple --runas options given"); runas = optarg; break; case 13: - if (logsize) { - err("Multiple --log-size options given"); - usage(argv[0]); - } + if (logsize) + errexit("Multiple --log-size options given"); errno = 0; logsize = strtol(optarg, NULL, 0); - if (logsize < LOGFILE_SIZE_MIN || errno) { - err("Invalid --log-size: %s", optarg); - usage(argv[0]); - } + if (logsize < LOGFILE_SIZE_MIN || errno) + errexit("Invalid --log-size: %s", optarg); + break; case 14: fprintf(stdout, @@ -1286,138 +1263,102 @@ void conf(struct ctx *c, int argc, char **argv) fprintf(stdout, VERSION_BLOB); exit(EXIT_SUCCESS); case 'd': - if (c->debug) { - err("Multiple --debug options given"); - usage(argv[0]); - } + if (c->debug) + errexit("Multiple --debug options given"); - if (c->quiet) { - err("Either --debug or --quiet"); - usage(argv[0]); - } + if (c->quiet) + errexit("Either --debug or --quiet"); c->debug = 1; break; case 'e': - if (logfile) { - err("Can't log to both file and stderr"); - usage(argv[0]); - } + if (logfile) + errexit("Can't log to both file and stderr"); - if (c->stderr) { - err("Multiple --stderr options given"); - usage(argv[0]); - } + if (c->stderr) + errexit("Multiple --stderr options given"); c->stderr = 1; break; case 'l': - if (c->stderr) { - err("Can't log to both stderr and file"); - usage(argv[0]); - } + if (c->stderr) + errexit("Can't log to both stderr and file"); - if (logfile) { - err("Multiple --log-file options given"); - usage(argv[0]); - } + if (logfile) + errexit("Multiple --log-file options given"); logfile = optarg; break; case 'q': - if (c->quiet) { - err("Multiple --quiet options given"); - usage(argv[0]); - } + if (c->quiet) + errexit("Multiple --quiet options given"); - if (c->debug) { - err("Either --debug or --quiet"); - usage(argv[0]); - } + if (c->debug) + errexit("Either --debug or --quiet"); c->quiet = 1; break; case 'f': - if (c->foreground) { - err("Multiple --foreground options given"); - usage(argv[0]); - } + if (c->foreground) + errexit("Multiple --foreground options given"); c->foreground = 1; break; case 's': - if (*c->sock_path) { - err("Multiple --socket options given"); - usage(argv[0]); - } + if (*c->sock_path) + errexit("Multiple --socket options given"); ret = snprintf(c->sock_path, UNIX_SOCK_MAX - 1, "%s", optarg); - if (ret <= 0 || ret >= (int)sizeof(c->sock_path)) { - err("Invalid socket path: %s", optarg); - usage(argv[0]); - } + if (ret <= 0 || ret >= (int)sizeof(c->sock_path)) + errexit("Invalid socket path: %s", optarg); + break; case 'F': - if (c->fd_tap >= 0) { - err("Multiple --fd options given"); - usage(argv[0]); - } + if (c->fd_tap >= 0) + errexit("Multiple --fd options given"); errno = 0; c->fd_tap = strtol(optarg, NULL, 0); - if (c->fd_tap < 0 || errno) { - err("Invalid --fd: %s", optarg); - usage(argv[0]); - } + if (c->fd_tap < 0 || errno) + errexit("Invalid --fd: %s", optarg); c->one_off = true; break; case 'I': - if (*c->pasta_ifn) { - err("Multiple --ns-ifname options given"); - usage(argv[0]); - } + if (*c->pasta_ifn) + errexit("Multiple --ns-ifname options given"); ret = snprintf(c->pasta_ifn, IFNAMSIZ - 1, "%s", optarg); - if (ret <= 0 || ret >= IFNAMSIZ - 1) { - err("Invalid interface name: %s", optarg); - usage(argv[0]); - } + if (ret <= 0 || ret >= IFNAMSIZ - 1) + errexit("Invalid interface name: %s", optarg); + break; case 'p': - if (*c->pcap) { - err("Multiple --pcap options given"); - usage(argv[0]); - } + if (*c->pcap) + errexit("Multiple --pcap options given"); ret = snprintf(c->pcap, sizeof(c->pcap), "%s", optarg); - if (ret <= 0 || ret >= (int)sizeof(c->pcap)) { - err("Invalid pcap path: %s", optarg); - usage(argv[0]); - } + if (ret <= 0 || ret >= (int)sizeof(c->pcap)) + errexit("Invalid pcap path: %s", optarg); + break; case 'P': - if (*c->pid_file) { - err("Multiple --pid options given"); - usage(argv[0]); - } + if (*c->pid_file) + errexit("Multiple --pid options given"); ret = snprintf(c->pid_file, sizeof(c->pid_file), "%s", optarg); - if (ret <= 0 || ret >= (int)sizeof(c->pid_file)) { - err("Invalid PID file: %s", optarg); - usage(argv[0]); - } + if (ret <= 0 || ret >= (int)sizeof(c->pid_file)) + errexit("Invalid PID file: %s", optarg); + break; case 'm': - if (c->mtu) { - err("Multiple --mtu options given"); - usage(argv[0]); - } + if (c->mtu) + errexit("Multiple --mtu options given"); errno = 0; c->mtu = strtol(optarg, NULL, 0); @@ -1427,11 +1368,9 @@ void conf(struct ctx *c, int argc, char **argv) break; } - if (c->mtu < ETH_MIN_MTU || c->mtu > (int)ETH_MAX_MTU || - errno) { - err("Invalid MTU: %s", optarg); - usage(argv[0]); - } + if (c->mtu < ETH_MIN_MTU || c->mtu > (int)ETH_MAX_MTU || errno) + errexit("Invalid MTU: %s", optarg); + break; case 'a': if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr) && @@ -1451,24 +1390,21 @@ void conf(struct ctx *c, int argc, char **argv) !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) break; - err("Invalid address: %s", optarg); - usage(argv[0]); + errexit("Invalid address: %s", optarg); break; case 'n': c->ip4.prefix_len = conf_ip4_prefix(optarg); - if (c->ip4.prefix_len < 0) { - err("Invalid netmask: %s", optarg); - usage(argv[0]); - } + if (c->ip4.prefix_len < 0) + errexit("Invalid netmask: %s", optarg); + break; case 'M': for (i = 0; i < ETH_ALEN; i++) { errno = 0; b = strtol(optarg + (intptr_t)i * 3, NULL, 16); - if (b < 0 || b > UCHAR_MAX || errno) { - err("Invalid MAC address: %s", optarg); - usage(argv[0]); - } + if (b < 0 || b > UCHAR_MAX || errno) + errexit("Invalid MAC address: %s", optarg); + c->mac[i] = b; } break; @@ -1486,41 +1422,30 @@ void conf(struct ctx *c, int argc, char **argv) !IN4_IS_ADDR_LOOPBACK(&c->ip4.gw)) break; - err("Invalid gateway address: %s", optarg); - usage(argv[0]); + errexit("Invalid gateway address: %s", optarg); break; case 'i': - if (ifi) { - err("Redundant interface: %s", optarg); - usage(argv[0]); - } + if (ifi) + errexit("Redundant interface: %s", optarg); - if (!(ifi = if_nametoindex(optarg))) { - err("Invalid interface name %s: %s", optarg, - strerror(errno)); - usage(argv[0]); - } + if (!(ifi = if_nametoindex(optarg))) + errexit("Invalid interface name %s: %s", optarg, + strerror(errno)); break; case 'D': if (!strcmp(optarg, "none")) { - if (c->no_dns) { - err("Redundant DNS options"); - usage(argv[0]); - } + if (c->no_dns) + errexit("Redundant DNS options"); - if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) { - err("Conflicting DNS options"); - usage(argv[0]); - } + if (dns4 - c->ip4.dns || dns6 - c->ip6.dns) + errexit("Conflicting DNS options"); c->no_dns = 1; break; } - if (c->no_dns) { - err("Conflicting DNS options"); - usage(argv[0]); - } + if (c->no_dns) + errexit("Conflicting DNS options"); if (dns4 - &c->ip4.dns[0] < ARRAY_SIZE(c->ip4.dns) && inet_pton(AF_INET, optarg, dns4)) { @@ -1534,29 +1459,22 @@ void conf(struct ctx *c, int argc, char **argv) break; } - err("Cannot use DNS address %s", optarg); - usage(argv[0]); + errexit("Cannot use DNS address %s", optarg); break; case 'S': if (!strcmp(optarg, "none")) { - if (c->no_dns_search) { - err("Redundant DNS search options"); - usage(argv[0]); - } + if (c->no_dns_search) + errexit("Redundant DNS search options"); - if (dnss != c->dns_search) { - err("Conflicting DNS search options"); - usage(argv[0]); - } + if (dnss != c->dns_search) + errexit("Conflicting DNS search options"); c->no_dns_search = 1; break; } - if (c->no_dns_search) { - err("Conflicting DNS search options"); - usage(argv[0]); - } + if (c->no_dns_search) + errexit("Conflicting DNS search options"); if (dnss - c->dns_search < ARRAY_SIZE(c->dns_search)) { ret = snprintf(dnss->n, sizeof(*c->dns_search), @@ -1568,8 +1486,7 @@ void conf(struct ctx *c, int argc, char **argv) break; } - err("Cannot use DNS search domain %s", optarg); - usage(argv[0]); + errexit("Cannot use DNS search domain %s", optarg); break; case '4': v4_only = true; @@ -1578,15 +1495,11 @@ void conf(struct ctx *c, int argc, char **argv) v6_only = true; break; case '1': - if (c->mode != MODE_PASST) { - err("--one-off is for passt mode only"); - usage(argv[0]); - } + if (c->mode != MODE_PASST) + errexit("--one-off is for passt mode only"); - if (c->one_off) { - err("Redundant --one-off option"); - usage(argv[0]); - } + if (c->one_off) + errexit("Redundant --one-off option"); c->one_off = true; break; @@ -1604,15 +1517,11 @@ void conf(struct ctx *c, int argc, char **argv) } } while (name != -1); - if (v4_only && v6_only) { - err("Options ipv4-only and ipv6-only are mutually exclusive"); - usage(argv[0]); - } + if (v4_only && v6_only) + errexit("Options ipv4-only and ipv6-only are mutually exclusive"); - if (*c->sock_path && c->fd_tap >= 0) { - err("Options --socket and --fd are mutually exclusive"); - usage(argv[0]); - } + if (*c->sock_path && c->fd_tap >= 0) + errexit("Options --socket and --fd are mutually exclusive"); ret = conf_ugid(runas, &uid, &gid); if (ret) @@ -1628,10 +1537,8 @@ void conf(struct ctx *c, int argc, char **argv) c->ifi4 = conf_ip4(ifi, &c->ip4, c->mac); if (!v4_only) c->ifi6 = conf_ip6(ifi, &c->ip6, c->mac); - if (!c->ifi4 && !c->ifi6) { - err("External interface not usable"); - exit(EXIT_FAILURE); - } + if (!c->ifi4 && !c->ifi6) + errexit("External interface not usable"); /* Inbound port options can be parsed now (after IPv4/IPv6 settings) */ optind = 1; -- 2.39.1