[Asterisk-code-review] acl.c: Improve ast ouraddrfor() diagnostic messages. (asterisk[certified/13.13])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/4667 )

Change subject: acl.c: Improve ast_ouraddrfor() diagnostic messages.
......................................................................


acl.c: Improve ast_ouraddrfor() diagnostic messages.

* Made not generate strings unless they will actually be used.

ASTERISK-26672

Change-Id: I155fbe7fdff5ce47dfe5326f3baf5446849702c3
---
M main/acl.c
1 file changed, 22 insertions(+), 14 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/main/acl.c b/main/acl.c
index 87776b3..9820e8b 100644
--- a/main/acl.c
+++ b/main/acl.c
@@ -914,40 +914,48 @@
 
 int ast_ouraddrfor(const struct ast_sockaddr *them, struct ast_sockaddr *us)
 {
+	/*
+	 * We must create the errno string before creating the address
+	 * string because it could wipe out errno on the error return
+	 * paths.
+	 */
+	const char *sock_err;
 	int port;
 	int s;
 
+	/* Preserve our original address port */
 	port = ast_sockaddr_port(us);
 
-	if ((s = socket(ast_sockaddr_is_ipv6(them) ? AF_INET6 : AF_INET,
-			SOCK_DGRAM, 0)) < 0) {
-		ast_log(LOG_ERROR, "Cannot create socket\n");
+	s = socket(ast_sockaddr_is_ipv6(them) ? AF_INET6 : AF_INET, SOCK_DGRAM, 0);
+	if (s < 0) {
+		sock_err = ast_strdupa(strerror(errno));
+		ast_log(LOG_ERROR, "Cannot create socket to %s: %s\n",
+			ast_sockaddr_stringify_addr(them), sock_err);
 		return -1;
 	}
 
 	if (ast_connect(s, them)) {
-		ast_log(LOG_WARNING, "Cannot connect\n");
+		sock_err = ast_strdupa(strerror(errno));
+		ast_log(LOG_WARNING, "Cannot connect to %s: %s\n",
+			ast_sockaddr_stringify_addr(them), sock_err);
 		close(s);
 		return -1;
 	}
 	if (ast_getsockname(s, us)) {
-
-		ast_log(LOG_WARNING, "Cannot get socket name\n");
+		sock_err = ast_strdupa(strerror(errno));
+		ast_log(LOG_WARNING, "Cannot get socket name for connection to %s: %s\n",
+			ast_sockaddr_stringify_addr(them), sock_err);
 		close(s);
 		return -1;
 	}
 	close(s);
 
-	{
-		const char *them_addr = ast_strdupa(ast_sockaddr_stringify_addr(them));
-		const char *us_addr = ast_strdupa(ast_sockaddr_stringify_addr(us));
-
-		ast_debug(3, "For destination '%s', our source address is '%s'.\n",
-				them_addr, us_addr);
-	}
-
 	ast_sockaddr_set_port(us, port);
 
+	ast_debug(3, "For destination '%s', our source address is '%s'.\n",
+		ast_strdupa(ast_sockaddr_stringify_addr(them)),
+		ast_strdupa(ast_sockaddr_stringify_addr(us)));
+
 	return 0;
 }
 

-- 
To view, visit https://gerrit.asterisk.org/4667
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I155fbe7fdff5ce47dfe5326f3baf5446849702c3
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>