git: 0648be66c477 - stable/13 - ping: Simplify protocol selection.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Wed, 13 Dec 2023 20:06:27 UTC
The branch stable/13 has been updated by des:
URL: https://cgit.FreeBSD.org/src/commit/?id=0648be66c4776de2e6490cbfe1f7857d1383bb06
commit 0648be66c4776de2e6490cbfe1f7857d1383bb06
Author: Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-10-10 22:47:46 +0000
Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-12-13 16:39:36 +0000
ping: Simplify protocol selection.
* Interrupt the option loop as soon as we have an indication of which
protocol is intended.
* If we end up having to perform a DNS lookup, loop over the entire
result looking for either IPv4 or IPv6 addresses.
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Reviewed by: rscheff, kevans, allanjude
Differential Revision: https://reviews.freebsd.org/D42137
(cherry picked from commit 7fd2c91a291bd518e012b438d6ca6fdd04d39dbf)
ping: Consistently use EX_NOHOST for DNS failures.
Traditionally, ping returned exit code EX_NOHOST if a DNS lookup failed.
That is still the case for the legacy code in the new merged ping, but
not for IPv6 targets, nor when a DNS lookup is performed in order to
determine which version of the tool to invoke.
While here, also make sure that the error message is consistent.
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D42159
(cherry picked from commit c4ffb80ef18f6b581dc28c14bc579e0e7c73438c)
ping: Add missing ATF boilerplate.
Reviewed by: kevans
Differential Revision: https://reviews.freebsd.org/D42161
(cherry picked from commit fc7143b48341fb16ef5b2262c7cd5b5c47056112)
---
sbin/ping/main.c | 129 ++++++++++++++++++++-----------------------
sbin/ping/ping6.c | 5 +-
sbin/ping/tests/ping_test.sh | 67 ++++++++++++++++++++--
3 files changed, 124 insertions(+), 77 deletions(-)
diff --git a/sbin/ping/main.c b/sbin/ping/main.c
index 6321178e1228..2e2d3170fd13 100644
--- a/sbin/ping/main.c
+++ b/sbin/ping/main.c
@@ -39,6 +39,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sysexits.h>
#include <unistd.h>
#include "main.h"
@@ -62,112 +63,102 @@
int
main(int argc, char *argv[])
{
-#if defined(INET) && defined(INET6)
+#if defined(INET)
struct in_addr a;
- struct in6_addr a6;
#endif
-#if defined(INET) || defined(INET6)
- struct addrinfo hints;
+#if defined(INET6)
+ struct in6_addr a6;
#endif
- int ch;
-#ifdef INET
- bool ipv4 = false;
+#if defined(INET) && defined(INET6)
+ struct addrinfo hints, *res, *ai;
+ const char *target;
+ int error;
#endif
-#ifdef INET6
- bool ipv6 = false;
+ int opt;
+#ifdef INET6
if (strcmp(getprogname(), "ping6") == 0)
- ipv6 = true;
+ return ping6(argc, argv);
#endif
- while ((ch = getopt(argc, argv, ":" OPTSTR)) != -1) {
- switch(ch) {
+ while ((opt = getopt(argc, argv, ":" OPTSTR)) != -1) {
+ switch (opt) {
#ifdef INET
case '4':
- ipv4 = true;
- break;
+ goto ping4;
#endif
#ifdef INET6
case '6':
- ipv6 = true;
- break;
+ goto ping6;
#endif
-#if defined(INET) && defined(INET6)
case 'S':
/*
* If -S is given with a numeric parameter,
* force use of the corresponding version.
*/
+#ifdef INET
if (inet_pton(AF_INET, optarg, &a) == 1)
- ipv4 = true;
- else if (inet_pton(AF_INET6, optarg, &a6) == 1)
- ipv6 = true;
- break;
+ goto ping4;
+#endif
+#ifdef INET6
+ if (inet_pton(AF_INET6, optarg, &a6) == 1)
+ goto ping6;
#endif
+ break;
default:
break;
}
}
+ /*
+ * For IPv4, only one positional argument, the target, is allowed.
+ * For IPv6, multiple positional argument are allowed; the last
+ * one is the target, and preceding ones are intermediate hops.
+ * This nuance is lost here, but the only case where it matters is
+ * an error.
+ */
if (optind >= argc)
usage();
- optreset = 1;
- optind = 1;
#if defined(INET) && defined(INET6)
- if (ipv4 && ipv6)
- errx(1, "-4 and -6 cannot be used simultaneously");
-#endif
-
-#if defined(INET) && defined(INET6)
- if (inet_pton(AF_INET, argv[argc - 1], &a) == 1) {
- if (ipv6)
- errx(1, "IPv6 requested but IPv4 target address "
- "provided");
+ target = argv[argc - 1];
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_socktype = SOCK_RAW;
+ if (feature_present("inet") && !feature_present("inet6"))
hints.ai_family = AF_INET;
- }
- else if (inet_pton(AF_INET6, argv[argc - 1], &a6) == 1) {
- if (ipv4)
- errx(1, "IPv4 requested but IPv6 target address "
- "provided");
+ if (feature_present("inet6") && !feature_present("inet"))
hints.ai_family = AF_INET6;
- } else if (ipv6)
- hints.ai_family = AF_INET6;
- else if (ipv4)
- hints.ai_family = AF_INET;
- else {
- if (!feature_present("inet6"))
- hints.ai_family = AF_INET;
- else if (!feature_present("inet"))
- hints.ai_family = AF_INET6;
- else {
- struct addrinfo *res;
-
- memset(&hints, 0, sizeof(hints));
- hints.ai_socktype = SOCK_RAW;
- hints.ai_family = AF_UNSPEC;
- getaddrinfo(argv[argc - 1], NULL, &hints, &res);
- if (res != NULL) {
- hints.ai_family = res[0].ai_family;
- freeaddrinfo(res);
- }
+ else
+ hints.ai_family = AF_UNSPEC;
+ error = getaddrinfo(target, NULL, &hints, &res);
+ if (res == NULL)
+ errx(EX_NOHOST, "cannot resolve %s: %s",
+ target, gai_strerror(error));
+ for (ai = res; ai != NULL; ai = ai->ai_next) {
+ if (ai->ai_family == AF_INET) {
+ freeaddrinfo(res);
+ goto ping4;
+ }
+ if (ai->ai_family == AF_INET6) {
+ freeaddrinfo(res);
+ goto ping6;
}
}
-#elif defined(INET)
- hints.ai_family = AF_INET;
-#elif defined(INET6)
- hints.ai_family = AF_INET6;
+ freeaddrinfo(res);
+ errx(EX_NOHOST, "cannot resolve %s", target);
#endif
-
#ifdef INET
- if (hints.ai_family == AF_INET)
- return ping(argc, argv);
-#endif /* INET */
+ping4:
+ optreset = 1;
+ optind = 1;
+ return ping(argc, argv);
+#endif
#ifdef INET6
- if (hints.ai_family == AF_INET6)
- return ping6(argc, argv);
-#endif /* INET6 */
- errx(1, "Unknown host");
+ping6:
+ optreset = 1;
+ optind = 1;
+ return ping6(argc, argv);
+#endif
}
void
diff --git a/sbin/ping/ping6.c b/sbin/ping/ping6.c
index 0edeb01b2dc0..d1f4c192b4c4 100644
--- a/sbin/ping/ping6.c
+++ b/sbin/ping/ping6.c
@@ -677,14 +677,15 @@ ping6(int argc, char *argv[])
error = cap_getaddrinfo(capdns, target, NULL, &hints, &res);
if (error)
- errx(1, "%s", gai_strerror(error));
+ errx(EX_NOHOST, "cannot resolve %s: %s",
+ target, gai_strerror(error));
if (res->ai_canonname)
hostname = strdup(res->ai_canonname);
else
hostname = target;
if (!res->ai_addr)
- errx(1, "cap_getaddrinfo failed");
+ errx(EX_NOHOST, "cannot resolve %s", target);
(void)memcpy(&dst, res->ai_addr, res->ai_addrlen);
diff --git a/sbin/ping/tests/ping_test.sh b/sbin/ping/tests/ping_test.sh
index ba0e38cb24b0..6cebdecc4f99 100644
--- a/sbin/ping/tests/ping_test.sh
+++ b/sbin/ping/tests/ping_test.sh
@@ -106,6 +106,7 @@ ping6_c1_s8_t1_body()
check_ping_statistics std.out $(atf_get_srcdir)/ping_6_c1_s8_t1.out
}
+atf_test_case ping_c1t6
ping_c1t6_head()
{
atf_set "descr" "-t6 is not interpreted as -t -6 by ping"
@@ -116,6 +117,7 @@ ping_c1t6_body()
atf_check -s exit:0 -o ignore -e empty ping -c1 -t6 127.0.0.1
}
+atf_test_case ping_c1t4
ping6_c1t4_head()
{
atf_set "descr" "-t4 is not interpreted as -t -4 by ping6"
@@ -126,6 +128,7 @@ ping6_c1t4_body()
atf_check -s exit:0 -o ignore -e empty ping6 -c1 -t4 ::1
}
+atf_test_case ping_46
ping_46_head()
{
atf_set "descr" "-4 and -6 cannot be used simultaneously"
@@ -135,21 +138,69 @@ ping_46_body()
require_ipv4
require_ipv6
atf_check -s exit:1 \
- -e match:"-4 and -6 cannot be used simultaneously" \
+ -e match:"illegal option -- 6" \
ping -4 -6 localhost
}
-ping6_46_head()
+atf_test_case ping_64
+ping_64_head()
{
atf_set "descr" "-4 and -6 cannot be used simultaneously"
}
-ping6_46_body()
+ping_64_body()
{
require_ipv4
require_ipv6
atf_check -s exit:1 \
- -e match:"-4 and -6 cannot be used simultaneously" \
- ping6 -4 -6 localhost
+ -e match:"illegal option -- 4" \
+ ping -6 -4 localhost
+}
+
+atf_test_case ping6_4
+ping6_4_head()
+{
+ atf_set "descr" "ping6 does not accept -4"
+}
+ping6_4_body()
+{
+ require_ipv4
+ require_ipv6
+ atf_check -s exit:1 \
+ -e match:"illegal option -- 4" \
+ ping6 -4 localhost
+}
+
+atf_test_case ping_nohost
+ping_nohost_head()
+{
+ atf_set "descr" "ping a nonexistent host"
+}
+ping_nohost_body()
+{
+ atf_check -s exit:68 -e match:"cannot resolve" \
+ ping nonexistent.in-addr.arpa.
+}
+
+atf_test_case ping4_nohost
+ping4_nohost_head()
+{
+ atf_set "descr" "ping -4 a nonexistent host"
+}
+ping4_nohost_body()
+{
+ atf_check -s exit:68 -e match:"cannot resolve" \
+ ping -4 nonexistent.in-addr.arpa.
+}
+
+atf_test_case ping6_nohost
+ping6_nohost_head()
+{
+ atf_set "descr" "ping -6 a nonexistent host"
+}
+ping6_nohost_body()
+{
+ atf_check -s exit:68 -e match:"cannot resolve" \
+ ping -6 nonexistent.in-addr.arpa.
}
atf_test_case "inject_opts" "cleanup"
@@ -212,7 +263,11 @@ atf_init_test_cases()
atf_add_test_case ping_c1t6
atf_add_test_case ping6_c1t4
atf_add_test_case ping_46
- atf_add_test_case ping6_46
+ atf_add_test_case ping_64
+ atf_add_test_case ping6_4
+ atf_add_test_case ping_nohost
+ atf_add_test_case ping4_nohost
+ atf_add_test_case ping6_nohost
atf_add_test_case inject_opts
atf_add_test_case inject_pip
atf_add_test_case inject_reply