[Asterisk-code-review] AMI: be less verbose when adding HTTP headers to AMI/HTTP me... (asterisk[master])

[Jaco Kroon]

Jaco Kroon has uploaded this change for review. ( https://gerrit.asterisk.org/10000


Change subject: AMI: be less verbose when adding HTTP headers to AMI/HTTP messages.
......................................................................

AMI: be less verbose when adding HTTP headers to AMI/HTTP messages.

All HTTP/AMI message headers are being sent to the verbose channel.
There are multiple places this is happening.  Consolidate the loop into
a function.  Drop the debug/verbose message.

Convert to using ast_asprintf to perform the length calculation, memory
allocation and snprintf all in one step.

Change-Id: Ic45e673fde05bd544be95ad5cdbc69518207c1a1
---
M main/manager.c
1 file changed, 34 insertions(+), 42 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/00/10000/1

diff --git a/main/manager.c b/main/manager.c
index a0dcf6c..2aa0989 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -2804,6 +2804,34 @@
 }
 
 /*!
+ * \brief Append additional headers into the message structure from params.
+ *
+ * \note You likely want to initialize m->hdrcount to 0 before calling this.
+ */
+static void astman_append_headers(struct message *m, const struct ast_variable *params)
+{
+	const struct ast_variable *v;
+
+	for (v = params; v && m->hdrcount < ARRAY_LEN(m->headers); v = v->next) {
+		if (ast_asprintf((char**)&m->headers[m->hdrcount], "%s: %s", v->name, v->value) < 0)
+			continue;
+		++m->hdrcount;
+	}
+}
+
+/*!
+ * \brief Free headers inside message structure, but not the message structure itself.
+ */
+static void astman_free_headers(struct message *m)
+{
+	while (m->hdrcount) {
+		--m->hdrcount;
+		ast_free((void *) m->headers[m->hdrcount]);
+		m->headers[m->hdrcount] = NULL;
+	}
+}
+
+/*!
  * \internal
  * \brief Process one "Variable:" header value string.
  *
@@ -6658,7 +6686,6 @@
 	struct message m = { 0 };
 	char header_buf[sizeof(s->session->inbuf)] = { '\0' };
 	int res;
-	int idx;
 	int hdr_loss;
 	time_t now;
 
@@ -6726,10 +6753,8 @@
 		}
 	}
 
-	/* Free AMI request headers. */
-	for (idx = 0; idx < m.hdrcount; ++idx) {
-		ast_free((void *) m.headers[idx]);
-	}
+	astman_free_headers(&m);
+
 	return res;
 }
 
@@ -7723,13 +7748,10 @@
 	uint32_t ident;
 	int fd;
 	int blastaway = 0;
-	struct ast_variable *v;
 	struct ast_variable *params = get_params;
 	char template[] = "/tmp/ast-http-XXXXXX";	/* template for temporary file */
 	struct ast_str *http_header = NULL, *out = NULL;
 	struct message m = { 0 };
-	unsigned int idx;
-	size_t hdrlen;
 
 	if (method != AST_HTTP_GET && method != AST_HTTP_HEAD && method != AST_HTTP_POST) {
 		ast_http_error(ser, 501, "Not Implemented", "Attempt to use unimplemented / unsupported method");
@@ -7812,17 +7834,7 @@
 		}
 	}
 
-	for (v = params; v && m.hdrcount < ARRAY_LEN(m.headers); v = v->next) {
-		hdrlen = strlen(v->name) + strlen(v->value) + 3;
-		m.headers[m.hdrcount] = ast_malloc(hdrlen);
-		if (!m.headers[m.hdrcount]) {
-			/* Allocation failure */
-			continue;
-		}
-		snprintf((char *) m.headers[m.hdrcount], hdrlen, "%s: %s", v->name, v->value);
-		ast_debug(1, "HTTP Manager add header %s\n", m.headers[m.hdrcount]);
-		++m.hdrcount;
-	}
+	astman_append_headers(&m, params);
 
 	if (process_message(&s, &m)) {
 		if (session->authenticated) {
@@ -7837,11 +7849,7 @@
 		session->needdestroy = 1;
 	}
 
-	/* Free request headers. */
-	for (idx = 0; idx < m.hdrcount; ++idx) {
-		ast_free((void *) m.headers[idx]);
-		m.headers[idx] = NULL;
-	}
+	astman_free_headers(&m);
 
 	ast_str_append(&http_header, 0,
 		"Content-type: text/%s\r\n"
@@ -7952,8 +7960,6 @@
 	struct ast_str *http_header = NULL, *out = NULL;
 	size_t result_size;
 	struct message m = { 0 };
-	unsigned int idx;
-	size_t hdrlen;
 	int fd;
 
 	time_t time_now = time(NULL);
@@ -8176,17 +8182,7 @@
 		}
 	}
 
-	for (v = params; v && m.hdrcount < ARRAY_LEN(m.headers); v = v->next) {
-		hdrlen = strlen(v->name) + strlen(v->value) + 3;
-		m.headers[m.hdrcount] = ast_malloc(hdrlen);
-		if (!m.headers[m.hdrcount]) {
-			/* Allocation failure */
-			continue;
-		}
-		snprintf((char *) m.headers[m.hdrcount], hdrlen, "%s: %s", v->name, v->value);
-		ast_verb(4, "HTTP Manager add header %s\n", m.headers[m.hdrcount]);
-		++m.hdrcount;
-	}
+	astman_append_headers(&m, params);
 
 	if (process_message(&s, &m)) {
 		if (u_displayconnects) {
@@ -8196,11 +8192,7 @@
 		session->needdestroy = 1;
 	}
 
-	/* Free request headers. */
-	for (idx = 0; idx < m.hdrcount; ++idx) {
-		ast_free((void *) m.headers[idx]);
-		m.headers[idx] = NULL;
-	}
+	astman_free_headers(&m);
 
 	result_size = lseek(ast_iostream_get_fd(s.stream), 0, SEEK_CUR); /* Calculate approx. size of result */
 

-- 
To view, visit https://gerrit.asterisk.org/10000
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic45e673fde05bd544be95ad5cdbc69518207c1a1
Gerrit-Change-Number: 10000
Gerrit-PatchSet: 1
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180823/57eba946/attachment-0001.html>