net: shell: dns: Properly manage lifetime of callback data

DNS callback needs "struct shell *shell" data structure to pass as
a parameter to shell print. How it was achieved previously is that
it was packaged together with cosmetic "bool first" param into
"struct net_shell_user_data" on the stack, and passed to the
callback. The problem was that the original command handler then
returned, so the "struct net_shell_user_data" on the stack was
overwritten, and the callback crashed on accessing it.

An obvious solution was to make that structure static, but that would
leave to issues still, as turns out we allow system shell to be run
as more than one concurrent instances.

Next solution was to keep this structure on the stack, but block the
command handler until callback is finished. However, that hit a
deadlock due to not well thought out use of a mutex in the shell
printing routines.

The solution presented here is due to @nordic-krch, who noticed that
"bool first" param is indeed cosmetic and not really required. Then
we have only "struct shell *shell" to pass to the callback, and can
do that in callback's pointer param directly, ditching
"struct net_shell_user_data" which needs to be stored on the stack.

Signed-off-by: Paul Sokolovsky <[email protected]>

Author: pfalcon

subsys/net/ip/net_shell.c

@@ -1299,24 +1299,16 @@ static void dns_result_cb(enum dns_resolve_status status,
 			  struct dns_addrinfo *info,
 			  void *user_data)
 {
-	struct net_shell_user_data *data = user_data;
-	const struct shell *shell = data->shell;
-	bool *first = data->user_data;
+	const struct shell *shell = user_data;
 
 	if (status == DNS_EAI_CANCELED) {
-		PR_WARNING("\nTimeout while resolving name.\n");
-		*first = false;
+		PR_WARNING("dns: Timeout while resolving name.\n");
 		return;
 	}
 
 	if (status == DNS_EAI_INPROGRESS && info) {
 		char addr[NET_IPV6_ADDR_LEN];
 
-		if (*first) {
-			PR("\n");
-			*first = false;
-		}
-
 		if (info->ai_family == AF_INET) {
 			net_addr_ntop(AF_INET,
 				      &net_sin(&info->ai_addr)->sin_addr,
@@ -1328,25 +1320,25 @@ static void dns_result_cb(enum dns_resolve_status status,
 		} else {
 			strncpy(addr, "Invalid protocol family",
 				sizeof(addr));
+			/* strncpy() doesn't guarantee NUL byte at the end. */
+			addr[sizeof(addr) - 1] = 0;
 		}
 
-		PR("\t%s\n", addr);
+		PR("dns: %s\n", addr);
 		return;
 	}
 
 	if (status == DNS_EAI_ALLDONE) {
-		PR("All results received\n");
-		*first = false;
+		PR("dns: All results received\n");
 		return;
 	}
 
 	if (status == DNS_EAI_FAIL) {
-		PR_WARNING("No such name found.\n");
-		*first = false;
+		PR_WARNING("dns: No such name found.\n");
 		return;
 	}
 
-	PR_WARNING("Unhandled status %d received\n", status);
+	PR_WARNING("dns: Unhandled status %d received\n", status);
 }
 
 static void print_dns_info(const struct shell *shell,
@@ -1449,10 +1441,8 @@ static int cmd_net_dns_query(const struct shell *shell, size_t argc,
 
 #if defined(CONFIG_DNS_RESOLVER)
 #define DNS_TIMEOUT K_MSEC(2000) /* ms */
-	struct net_shell_user_data user_data;
 	enum dns_query_type qtype = DNS_QUERY_TYPE_A;
 	char *host, *type = NULL;
-	bool first = true;
 	int ret, arg = 1;
 
 	host = argv[arg++];
@@ -1479,11 +1469,8 @@ static int cmd_net_dns_query(const struct shell *shell, size_t argc,
 		}
 	}
 
-	user_data.shell = shell;
-	user_data.user_data = &first;
-
-	ret = dns_get_addr_info(host, qtype, NULL, dns_result_cb, &user_data,
-				DNS_TIMEOUT);
+	ret = dns_get_addr_info(host, qtype, NULL, dns_result_cb,
+				(void *)shell, DNS_TIMEOUT);
 	if (ret < 0) {
 		PR_WARNING("Cannot resolve '%s' (%d)\n", host, ret);
 	} else {