git: 0648be66c477 - stable/13 - ping: Simplify protocol selection.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
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